Skip to content

Conversation

@leplatrem
Copy link
Contributor

@leplatrem leplatrem commented Aug 31, 2022

fixes #213

I removed some of the class-based stuff, but not everything, we should agree on a design for #77 before we can go further IMO

@leplatrem leplatrem force-pushed the 213-split-default-action-into-functions branch from f2dcb06 to 5fb2110 Compare August 31, 2022 13:30
@leplatrem leplatrem marked this pull request as ready for review August 31, 2022 13:45
@leplatrem leplatrem requested a review from a team as a code owner August 31, 2022 13:45
Copy link
Contributor

@bsieber-mozilla bsieber-mozilla left a comment

Choose a reason for hiding this comment

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

Thanks for the efforts in this PR and #233.

I think we can land this and iterate on this "pipeline" approach we discussed in the meeting today.

Copy link
Contributor

@grahamalama grahamalama 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'm approving this knowing that #233 is a fast follow and we'll be quickly iterating on this design, but these were just things I wanted to comment on for posterity if for nothing else.

@leplatrem
Copy link
Contributor Author

Looks good! I'm approving this knowing that #233 is a fast follow

Nice! I will #233 and address your comments then.

Maybe we should cut a release before landing this beast on main though

@grahamalama
Copy link
Contributor

Maybe we should cut a release before landing this beast on main though

That's a good idea!

leplatrem and others added 5 commits September 6, 2022 16:04
…ors) (#236)

* Rename ActionLogContext to ActionContext

* Rename log_context to context

* Remove := operator and chain operations

* Remove the maybe_ from services.jira

* Align signatures of action functions

* Do not duplicate parameters between init and call

* Runner now instantiates context

* Pass actions contexts to action tests

* Update actions docs

* Remove redundant parameters

* Adjust parameters between default.py and jira.py

* Inherit JiraContext from Context

* Remove useless and confusing condition

* Replace repetitive calls with list of steps

* Let the runner set the ActionContext operation

* Reintroduce maybe_ for optional steps

* Load default action steps from configuration (#253)

* Move steps to jbi.actions.steps

* Rename test_default to test_steps

* Move test_default_with_assignee_and_status into test_steps

* Load default action steps from configuration

* Add tests for the default action behaviour

* Rename groups of steps. create -> new, update -> existing

* Remove default value config/config.prod.yaml

Co-authored-by: bsieber-mozilla <[email protected]>

* Move remapping of steps into helper

* Remove useless pylint annotation

* Adjust docstrings

* Merge unspecified groups with default ones

* Update secrets baseline

Co-authored-by: bsieber-mozilla <[email protected]>

Co-authored-by: bsieber-mozilla <[email protected]>
@grahamalama
Copy link
Contributor

Before we merge this, could we update the title / description to reflect the fact that it's an "omnibus" PR that includes #233 and #253?

@leplatrem leplatrem changed the title Split default action into smaller pieces (fixes #213) Split actions into reusable steps (fixes #213, fixes #77, embeds #233, #236, #253) Oct 5, 2022
@leplatrem leplatrem merged commit 0da6d79 into main Oct 5, 2022
@leplatrem leplatrem deleted the 213-split-default-action-into-functions branch October 5, 2022 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make the default action (and the logic contained inside it) easier to reuse

4 participants