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.
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
fluff: Allow for a list of custom edit summaries #1283
fluff: Allow for a list of custom edit summaries #1283
Changes from 2 commits
76b167d
3caa3ab
f20dbaf
2fab1aa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Not sure about this. Would it make more sense to make this list be empty and allow users to provide it themselves, then offer a few options (such as these?) in addition to that list? Kind of like the other customizations (warn/tag/welcome templates). It'd be annoying if you want to remove one, but I think a dropdown would help with that (though see other comment).
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.
Agreed this looks a bit silly. Not so much agreed on the solution. I see that twinkleconfig is missing a "plainlist" data type that would have been the natural solution here. It might be worth adding that just for this usecase.
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.
We'd have to prepopulate it with something, no?
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.
There are two things to discuss here:
One is how to store the data. A
plainlist
would be nice, but I don't thinkcustomlist
is a huge hindrance. The latter might make the display easier to parse for folks if there are multiple options. So maybe "sources: removed reliable sources" and "sources: needs reliable sources" or something. A plainlist does seem simpler given we just need one string of text.The other is how to incorporate defaults, which I'm more concerned about. With this setup, if someone customizes the list, they are forever left out of any changes we make unless they hit "reset, view the default options, then re-add from memory what they wanted; that's not good. Instead we can do what is done elsewhere, namely include the basic uses in the module, and then add any items from
customReversionSummaryList
on top of that. The advantage of the former is that users could cull the defaults, though.