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.

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

revision of AEP "status" codes #22

Closed
ltalirz opened this issue Oct 12, 2020 · 3 comments · Fixed by #23
Closed

revision of AEP "status" codes #22

ltalirz opened this issue Oct 12, 2020 · 3 comments · Fixed by #23

Comments

@ltalirz
Copy link
Member

ltalirz commented Oct 12, 2020

We've started out with the status codes used also by the jupyter project:

  • submitted - this should be the initial status when submitting the pull request to the AEP repository
  • active - this AEP has been accepted and people are actively discussing and implementing it
  • implemented - this AEP has been implemented
  • postponed - this AEP is no longer active, might be interesting for the project but has noone willing to champion it
  • rejected - this AEP has been rejected and will not be implemented
  • withdrawn - this AEP has been withdrawn by the submitter but can be re-submitted if someone is willing to champion

In the context of discussions concerning PRs #11 and #21 a question arose about the right time to merge a PR:
Does it make sense to merge a PR once people have agreed to move forward in this direction or should a PR be merged only once all implementation details have been figured out (which, effectively, is after its implementation in aiida-core)?

The general feeling seemed to be that it would be useful to merge PRs in a draft stage as an intermediate form of saying "we agree that we want this to happen".
As it turns out, this also came up in the jupyter project jupyter/governance#13 (but has so far gone stale).

Alternatives for inspiration:

@ltalirz
Copy link
Member Author

ltalirz commented Oct 12, 2020

It seems to me that we could achieve our goal with the following minimal changes:

  • Rename status active to draft (in essence, if people are working on and implementing it, then this is the draft state)
  • Remove the submitted status (no PR will ever be accepted with status submitted - it seems pointless to me to force changing of this status during peer review)

Let me know what you think via up/downvotes and please feel free to comment

@greschd
Copy link
Member

greschd commented Oct 12, 2020

During the draft stage, is the idea that the people implementing it continually update the AEP (via PRs) if there are major changes to the design? And then it goes directly from draft to implemented?

@ltalirz
Copy link
Member Author

ltalirz commented Oct 12, 2020

During the draft stage, is the idea that the people implementing it continually update the AEP (via PRs) if there are major changes to the design?

If there is a major change to the design, then an update of the AEP would seem to be warranted. In practice, I think people will be lazy in updating the draft AEPs and I don't expect this to happen very often.

And then it goes directly from draft to implemented?

Once it is implemented, yes.

P.S. Just to briefly explain how this could improve upon the current approach:
Especially with larger projects it can take a significant amount of time (easily a couple of months) between discussions on a first concept and the actual implementation/release in aiida-core.
With the current approach, you will typically see some initial discussion in a PR, which then dies at some point. Since there is no "checkpointing", when looking back at the PR after a long time, it may be unclear what was actually agreed upon and which questions were left open. Agreeing to merge the PR in a draft state would provide such a checkpoint (besides making the draft more visible in the AEP repository).

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 a pull request may close this issue.

2 participants