-
Notifications
You must be signed in to change notification settings - Fork 378
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
feat: remove "AD is watching" state #7960
feat: remove "AD is watching" state #7960
Conversation
Created as draft because there are still a bunch of references to the |
In deciding what to do with docs in the "AD is watching" state (of which there were 60 as of a ~week-old dev DB snapshot), I'm running into #5560 - my first pass at the migration put expired drafts into |
yes, many transitions to dead should now be transitions to idexist. I can't think of any place we should have code choosing the dead state - an AD should have to choose that explicitly. The code that does is leftover from before we made an idexists state for the draft-iesg state machine |
by=system_person, | ||
doc=doc, | ||
rev=doc.rev, | ||
desc=f"{draft_iesg_state_type.label} changed to <b>{idexists_state.name}</b> from {watching_state.name}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cringing at the html tags, but keeping the set of things to mutate later regular was a good call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I share the cringe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once guidance to new ads change requested.
This document is in IESG state "Dead". It is unusual to change | ||
this to anything other than "AD is watching", and this should | ||
never be used as a replacement for Begin IESG Processing. | ||
This document is in IESG state "Dead". It is unusual to change this to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add back the "never be used as a replacement for Begin IESG Processing". (New IESG members assume that moving things into pubreq will have all the side-effects that the form/view for Begin IESG Processing applies, but that's not possible.
We could also consider not allowing the document to make any other transition in this particular case.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7960 +/- ##
==========================================
+ Coverage 88.78% 88.84% +0.05%
==========================================
Files 296 304 +8
Lines 41320 41527 +207
==========================================
+ Hits 36687 36893 +206
- Misses 4633 4634 +1 ☔ View full report in Codecov by Sentry. |
Migrates any docs in the
watching
state toidexists
, updating their histories and adding a comment to their event log. Removes all references to thewatching
state from the code. Removes thecreate_in_state
choice field from theedit_info()
view used to begin the IESG process and instead uses thepub-req
state exclusively.Fixes #6316
Fixes #7583