Skip to content

Conversation

@lysnikolaou
Copy link
Member

@lysnikolaou lysnikolaou commented May 13, 2019

Upon changing the status to awaiting change review, a review is also requested through the new GitHub API from the appropriate reviewers.

#165

@codecov
Copy link

codecov bot commented May 13, 2019

Codecov Report

Merging #168 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #168    +/-   ##
======================================
  Coverage     100%   100%            
======================================
  Files          20     20            
  Lines        1557   1663   +106     
  Branches       87     89     +2     
======================================
+ Hits         1557   1663   +106
Impacted Files Coverage Δ
bedevere/stage.py 100% <100%> (ø) ⬆️
tests/test_stage.py 100% <100%> (ø) ⬆️
bedevere/bpo.py 100% <0%> (ø) ⬆️
tests/test_bpo.py 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6147b3f...fb7063f. Read the comment docs.

@brettcannon brettcannon self-requested a review May 23, 2019 22:28
Copy link
Member

@brettcannon brettcannon left a comment

Choose a reason for hiding this comment

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

One very minor change and a clarifying question based on whether you have live-tested this or not.

Co-Authored-By: Brett Cannon <54418+brettcannon@users.noreply.github.com>
@Mariatta
Copy link
Member

Mariatta commented Jul 8, 2019

Thanks @lysnikolaou for working on this. Would you be able to update the state machine at the top of stage.py to reflect the updated workflow?
I've just remembered it as I try to add "triagers" into the mix.

@lysnikolaou
Copy link
Member Author

@Mariatta I guess I could do that, but I don't understand what you mean by "updated workflow"? If you mean the new GitHub Triagers, then I don't know what the new state should be.

@brettcannon
Copy link
Member

@lysnikolaou I think she means what you're changing with this PR.

@lysnikolaou
Copy link
Member Author

@brettcannon @Mariatta The thing is that the state machine should fundamentally remain the same. Maybe we could change the label on the edge Awaiting Changes -> Awaiting change review to include something like "New review gets requested", but I don't see what other changes need to be made.

@brettcannon
Copy link
Member

@lysnikolaou yep, probably just the edge label to denote the side-effect that occurs.

@brettcannon brettcannon self-requested a review July 26, 2019 20:07
@lysnikolaou
Copy link
Member Author

@brettcannon I have updated the edge label, that needed tweaking, so the PR might be ready for another review.

@brettcannon
Copy link
Member

LGTM! @Mariatta did you want to do one final review are you okay with merging this?

@Mariatta
Copy link
Member

Sorry I don't have bandwidth to review this at the moment, don't want to block the progress.

@lysnikolaou
Copy link
Member Author

@brettcannon Can this be merged as well?

Co-Authored-By: Brett Cannon <54418+brettcannon@users.noreply.github.com>
@brettcannon brettcannon merged commit 6ea143b into python:master Sep 10, 2019
@brettcannon
Copy link
Member

Thanks for doing this, @lysnikolaou !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants