Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify logic in SCI plugin #743

Merged
merged 12 commits into from
May 31, 2024
Merged

Simplify logic in SCI plugin #743

merged 12 commits into from
May 31, 2024

Conversation

jmcook1186
Copy link
Contributor

Types of changes

  • Enhancement (project structure, spelling, grammar, formatting)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

A description of the changes proposed in the Pull Request

Applies simplified logic to SCI plugin and updates associated documentation, tests and manifests.

@jmcook1186 jmcook1186 self-assigned this May 30, 2024
narekhovhannisyan
narekhovhannisyan requested changes May 31, 2024
View reviewed changes
Copy link
Member

@narekhovhannisyan narekhovhannisyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix the requested changes above

manushak
manushak reviewed May 31, 2024
View reviewed changes
src/builtins/sci/README.md Outdated Show resolved Hide resolved
src/builtins/sci/index.ts Outdated Show resolved Hide resolved
@jmcook1186
Update manifests/examples/nesting.yml
c36bd6c 
Signed-off-by: Joseph Cook <33655003+jmcook1186@users.noreply.github.com>
@jmcook1186
Update src/builtins/sci/README.md
98d6db3 
Signed-off-by: Joseph Cook <33655003+jmcook1186@users.noreply.github.com>
@jmcook1186
feat(lib): add check that functional unit value is +ve int
b543961
@jmcook1186
feat(lib): add unit test to cover negative functional unit value
d4c8abf
@jmcook1186
Merge branch 'main' into simplify-sci
249499c 
Signed-off-by: Joseph Cook <33655003+jmcook1186@users.noreply.github.com>
manushak
manushak reviewed May 31, 2024
View reviewed changes
manifests/examples/pipeline-teads-sci.yml Outdated Show resolved Hide resolved
manifests/examples/pipeline-with-aggregate.yml Outdated Show resolved Hide resolved
manifests/examples/pipeline-with-mocks.yml Outdated Show resolved Hide resolved
@jmcook1186
Apply suggestions from code review
188e5e7 
Signed-off-by: Joseph Cook <33655003+jmcook1186@users.noreply.github.com>
@jmcook1186 jmcook1186 merged commit 49205ff into main May 31, 2024
2 checks passed
@jmcook1186 jmcook1186 deleted the simplify-sci branch May 31, 2024 18:33
@jmcook1186 jmcook1186 mentioned this pull request Jun 5, 2024
Open
@github-actions github-actions bot mentioned this pull request Jun 11, 2024
Merged
@github-actions github-actions bot mentioned this pull request Jun 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@narekhovhannisyan narekhovhannisyan narekhovhannisyan approved these changes

@manushak manushak manushak approved these changes

@MariamKhalatova MariamKhalatova Awaiting requested review from MariamKhalatova

Assignees

@jmcook1186 jmcook1186

Labels
None yet
Projects
IF
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jmcook1186 @manushak @narekhovhannisyan