Charcoal-SE / metasmoke

Web dashboard for SmokeDetector.
https://metasmoke.erwaysoftware.com
Creative Commons Zero v1.0 Universal
43 stars 34 forks source link

Rename 'Code Admin' role to 'Blacklist manager' #318

Closed angussidney closed 4 years ago

angussidney commented 6 years ago

Scroll down for up-for-grabs summary and guidance.

I'm proposing that we rename the code admin role to 'Blacklist manager' or similar.

Originally, this privilege was only given to people who had push access to the repo on Github (hence the name 'code admin'). But once the blacklist command was introduced, and we wanted to allow non-collaborators to edit the blacklist too, this definition quickly broke down.

Today, the role is used almost exclusively for blacklisting. The only privilege which would make more sense for an actual code admin is the ability to failover.

πŸ‘ to rename it, πŸ‘Ž to leave it, and comment if you have other ideas.

ArtOfCode- commented 6 years ago

Indifferent. A rename makes more semantic sense, but the current name isn't causing problems beyond a bit of confusion on occasion.

angussidney commented 6 years ago

It's a relatively simple change to make, so I'm willing to do it.

iBug commented 6 years ago

Not sure if this is a big change.

Currently the only thing Blacklist Maintainers can do is issue new keywords and approve non-code-privileged users' issues. If one accidentally made a typo when blacklisting, or if one needs to merge or delete a blacklist item, they need to propose a PR and wait for it to be merged by a real code admin.

So, we could extend this, probably by allowing BMs to commit directly to the blacklist .txt files. This is not possible as per the design of GitHub, so I think we could allow a bot that merges the PRs from BMs automatically, if only the blacklist files are changed. This way we are really getting "Maintainers" (instead of "Blacklist Adders").

ArtOfCode- commented 6 years ago

I'm sure we looked at something like that... Not sure where it got to

angussidney commented 6 years ago

Honestly, I don’t think it’s worth setting up a bot like that since Helios will allow the same functionality once it is implemented.

angussidney commented 6 years ago

Looks like a good hacktoberfest issue

ArtOfCode- commented 6 years ago

Contributor Guidance

Summary

Difficulty: 2/5 stars

This issue involves renaming one of the permissions in Charcoal's systems. It's currently called "code admin", but that doesn't reflect what it actually is very well, so we'd like to update it to "blacklist manager".

For the most part this should be relatively easy find-and-replace, but there are places where you'll need to take care that other things relying on the current name don't break. You'll also need to rename the permission across multiple Charcoal projects - not only this one (metasmoke), but also SmokeDetector and our website repository.

Guidance

You'll need to rename the permission in several places. I'd suggest starting off with the permission itself (which you'll find in Role#names, in app/models/role.rb). Once you've done that, have a look for other occurrences - that might be role checks, before_actions, method names, etc. Variable names aren't as important to update as method names, but would still be awesome to do if you have the time. You'll also need to update references to any methods you rename.

If you want to leave it there, send in a pull request to this repository and we'll review it together. If you'd like to carry on, you can also rename the permission in other places we use it - SmokeDetector is the most important, followed by documentation and so on in our website. If you need more guidance about those two repositories, let this issue's owner (listed above) know in this or any related thread.

How to get help

For additional support in completing this issue, either:

snpd25 commented 5 years ago

@ArtOfCode- , Can you please help to know which file needs modification

ArtOfCode- commented 5 years ago

@snpd25 There are lots. Have a read of the Guidance section in my comment above, that should give you an idea of where to start, but it won't be a single file - there are multiple files across several projects.

snpd25 commented 5 years ago

@ArtOfCode- Few migrations are also required to be run, right?

thesecretmaster commented 4 years ago

Seeing as this PR has been merged, is this status-complete?

teward commented 4 years ago

Closed by #682