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

feat: adding new node type CompositeActionInput #133

Merged
merged 4 commits into from
Nov 10, 2023
Merged

feat: adding new node type CompositeActionInput #133

merged 4 commits into from
Nov 10, 2023

Conversation

elad-pticha
Copy link
Contributor

No description provided.

@elad-pticha elad-pticha changed the title feat: adding new node type: compositeactioninput feat: adding new node type compositeactioninput Nov 9, 2023
@elad-pticha elad-pticha changed the title feat: adding new node type compositeactioninput feat: adding new node type CompositeActionInput Nov 9, 2023
@elad-pticha
Copy link
Contributor Author

elad-pticha commented Nov 9, 2023 •
edited
Loading

Until now, for each composite action, we created a list of inputs. This list did not contain a default value or if the input is required or not.

We should be able to query all CompositeActions that use a github context by the default value or just filter by other parameters.
With the new CompositeActionInput node, we will be able to filter all composite actions that use a specific github context.

image

@elad-pticha elad-pticha force-pushed the feat/composite-action-input-node branch 7 times, most recently from 8700bd4 to a56c5ca Compare November 9, 2023 13:10
@elad-pticha elad-pticha force-pushed the feat/composite-action-input-node branch from a56c5ca to 757ca2b Compare November 9, 2023 13:15
@elad-pticha elad-pticha force-pushed the feat/composite-action-input-node branch from 757ca2b to a7e688b Compare November 9, 2023 13:18
alex-ilgayev
alex-ilgayev approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@alex-ilgayev alex-ilgayev left a comment

Choose a reason for hiding this comment

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

LGTM!

oreenlivnicode
oreenlivnicode suggested changes Nov 9, 2023
View reviewed changes
.github/workflows/test_pr.yml Show resolved Hide resolved
tests/utils.py Outdated Show resolved Hide resolved
src/common/utils.py Show resolved Hide resolved
tests/utils.py Show resolved Hide resolved
src/workflow_components/composite_action.py Show resolved Hide resolved
src/workflow_components/composite_action.py Outdated Show resolved Hide resolved
src/workflow_components/composite_action.py Outdated Show resolved Hide resolved
tests/integration/test_graph_structures.py Outdated Show resolved Hide resolved
tests/utils.py Outdated Show resolved Hide resolved
tests/utils.py Show resolved Hide resolved
oreenlivnicode
oreenlivnicode reviewed Nov 9, 2023
View reviewed changes
Copy link
Contributor

@oreenlivnicode oreenlivnicode left a comment

Choose a reason for hiding this comment

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

Looks good, I requested some convention changes.

Does it add a lot of overhead to the indexing process caused by a lot of relationships from actions to inputs?

@elad-pticha elad-pticha force-pushed the feat/composite-action-input-node branch from 598a478 to f3aff1e Compare November 9, 2023 18:01
@elad-pticha elad-pticha merged commit 38ff344 into main Nov 10, 2023
@elad-pticha elad-pticha deleted the feat/composite-action-input-node branch November 10, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alex-ilgayev alex-ilgayev alex-ilgayev approved these changes

@oreenlivnicode oreenlivnicode oreenlivnicode approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@elad-pticha @alex-ilgayev @oreenlivnicode