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

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

Closed
angussidney opened this issue Feb 18, 2018 · 12 comments
Closed

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

angussidney opened this issue Feb 18, 2018 · 12 comments
Labels
status: completed Resolved/fixed. type: feature request Requests for a new feature.

Comments

@angussidney
Copy link
Member

angussidney commented Feb 18, 2018

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-
Copy link
Member

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

@angussidney
Copy link
Member Author

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

@iBug
Copy link
Member

iBug commented Feb 18, 2018

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-
Copy link
Member

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

@angussidney
Copy link
Member Author

angussidney commented Feb 18, 2018

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.

@ArtOfCode- ArtOfCode- added status: deferred We'll do it later. Maybe. 6-8 months. and removed refactor efforts labels Aug 12, 2018
@angussidney
Copy link
Member Author

Looks like a good hacktoberfest issue

@angussidney angussidney reopened this Sep 5, 2018
@angussidney angussidney added the hacktoberfest HF issues label. Outside of HF, also consider using type: up-for-grabs. label Sep 5, 2018
@ArtOfCode-
Copy link
Member

ArtOfCode- commented Sep 5, 2018

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:

  • comment on this thread
  • if you have a Stack Overflow account, drop into our chatroom (you'll need 20 reputation - if you don't have that, let me know your username or ID here and I'll grant access manually)

@ArtOfCode- ArtOfCode- removed the status: deferred We'll do it later. Maybe. 6-8 months. label Sep 5, 2018
@ArtOfCode- ArtOfCode- added the type: up-for-grabs Issues available for people new to open source. Detailed walkthrough and support is available. label Oct 25, 2018
@ArtOfCode- ArtOfCode- added status: deferred We'll do it later. Maybe. 6-8 months. and removed hacktoberfest HF issues label. Outside of HF, also consider using type: up-for-grabs. labels Nov 1, 2018
@snpd25
Copy link
Contributor

snpd25 commented Oct 15, 2019

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

@ArtOfCode-
Copy link
Member

@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
Copy link
Contributor

snpd25 commented Oct 16, 2019

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

@thesecretmaster
Copy link
Member

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

@teward
Copy link
Member

teward commented Jul 8, 2020

Closed by #682

@teward teward closed this as completed Jul 8, 2020
@teward teward added status: completed Resolved/fixed. and removed status: deferred We'll do it later. Maybe. 6-8 months. type: up-for-grabs Issues available for people new to open source. Detailed walkthrough and support is available. labels Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: completed Resolved/fixed. type: feature request Requests for a new feature.
Development

No branches or pull requests

6 participants