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

Update AEP 000: Add draft status #23

Merged
merged 6 commits into from
Dec 17, 2021

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Nov 1, 2020

Fixes #22

Especially with larger projects it can take a significant amount of time
(easily a couple of months) between first discussions on an enhancement
proposal and its actual implementation/release in aiida-core. With the
current approach, we have typically seen some initial discussion in a
pull request, which then died at some point.

Since there is no "checkpointing", when looking back at the PR after a
long time, it can be unclear what was actually agreed upon and which
questions were left open. Agreeing to merge the PR in a draft state
provides such a checkpoint (besides making the draft more visible
in the AEP repository).

We also remove the 'submitted' status, since it was used only during
the pull request stage and always needed to be changed before merging.

000_aep_guidelines/readme.md Outdated Show resolved Hide resolved
@chrisjsewell chrisjsewell mentioned this pull request Jan 10, 2021
5 tasks
@sphuber sphuber changed the title add 'draft' status Update AEP 000: Add draft status Dec 16, 2021
Especially with larger projects it can take a significant amount of time
(easily a couple of months) between first discussions on an enhancement
proposal and its actual implementation/release in aiida-core.  With the
current approach, we have typically seen some initial discussion in a
pull request, which then died at some point.

Since there is no "checkpointing", when looking back at the PR after a
long time, it can be unclear what was actually agreed upon and which
questions were left open. Agreeing to merge the PR in a draft state
provides such a checkpoint (besides making the draft more visible
in the AEP repository).

We also remove the 'submitted' status, since it was used only during
the pull request stage and always needed to be changed before merging.
Co-authored-by: Carl Simon Adorf <carl.simon.adorf@gmail.com>
@ltalirz
Copy link
Member Author

ltalirz commented Dec 16, 2021

Given that AEP 005 was already merged in draft status and we're currently lacking a formal description of it, let's move forward with this.

I've accepted the suggestion by @csadorf for the description of the draft status; I no longer feel as strongly about the wording as I did before, and I think is a good step forward.

@ltalirz ltalirz requested a review from csadorf December 16, 2021 23:12
@ltalirz
Copy link
Member Author

ltalirz commented Dec 16, 2021

@sphuber please have a look; also pinged @csadorf

+ clarify meaning for "created date"
Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

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

Thanks @ltalirz , phrasing is fine for me. Just some small corrections to make and wondering if we should have a date that records day of merging the AEP, i.e., when it became accepted as a draft

000_aep_guidelines/readme.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
000_aep_guidelines/readme.md Outdated Show resolved Hide resolved
004_calcjob_importer/readme.md Show resolved Hide resolved
Copy link
Contributor

@csadorf csadorf left a comment

Choose a reason for hiding this comment

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

I'm going to approve this on the condition that the commit is cleaned up (@sphuber pointed out some artifacts). Other than that, I think this is fine as-is. I would recommend against adding the "date of modification" field, but I don't feel strongly about it and would find it more important that this PR is finally merged.

ltalirz and others added 3 commits December 17, 2021 10:41
Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
@ltalirz
Copy link
Member Author

ltalirz commented Dec 17, 2021

@sphuber Would you like to do a final check?

@sphuber sphuber merged commit 37c934f into aiidateam:master Dec 17, 2021
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.

revision of AEP "status" codes
4 participants