-
Notifications
You must be signed in to change notification settings - Fork 120
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
Clarify expectations about FO mitigations #659
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why is this a MAY for mature publications but a MUST for general transition requirements?
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.
It is "may" for updating a publication while staying at the same status, like CR->CR, because you may be in the process of dealing with issues. If you've got a pile of 30 issues, and you've fixed 15 of them, you have 15 left and they're still problems, but you've made progress, so republishing is an improvement. So, if you're ignoring FOs, you should be denied. If you're working through a pile of things, and you're not done with all of them yet, then it's fine.
For a transition to a more mature stage, you're actually supposed to have cleared the deck, so it's a "must" in that case.
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.
Right, that makes sense (with one exception), thanks @frivoal - unfortunately the heading "6.3.4. Updating Mature Publications on the Recommendation Track" is suggestive of specifications at the higher end of maturity, rather than all stages. It could be a separate ticket, but renaming the heading would avoid this. E.g. "Updating Recommendation Track Publications without advancement".
The exception is Rec -> Rec transition, where I'd expect something like the streamlined requirements (which talk about issues, of which I think FOs are a subset) to apply even for team verification.
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.
Ah, right, I see. This is tricky, because the criteria for this doesn't apply to WD->WD transition. "Updating Recommendation Track Publications without advancement" as you suggested implies it would apply to all updates, not just certain ones as clarified in the opening paragraph. I suspect renaming could be useful for the reason you highlighted, but I don't think we've found the right name just yet.
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.
Oh, I see that I was even more confused than I first realised when looking at this text. The text in §6.3.4 says "Certain requests" and does not define which ones, and then defines the term "Update requests", which is then referenced from elsewhere, which is where these are brought in normatively.
Maybe this subsection should be called "Requirements for Update Requests" and there could be a note that says that the option to make Update Requests is defined by reference from the relevant sections. Right now update request requirements need to be met for:
And they are called out as not being needed for CRD -> CRD and Registry Table updates.
Given that, we could state, even in a note, within §6.3.4, that those "certain categories" are exactly the two I listed above. I think it would help avoid confusion for other readers.
Could it be helpful to define a term for the special nature of CRS and Rec as being normative documents for which we expect/permit there to be normative references from other documents?
(sidenote: I still don't think normative references to CR are a good idea, but the decision to permit them is now ancient history)
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.
@nigelmegitt Can you file your comments here as a separate issue? :) I think your suggestions to clarify this make sense, but they're about clarifying pre-existing text.