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

Moderation info overhaul #119

Open
TheCakeIsNaOH opened this issue Jan 9, 2021 · 29 comments
Open

Moderation info overhaul #119

TheCakeIsNaOH opened this issue Jan 9, 2021 · 29 comments
Assignees

Comments

@TheCakeIsNaOH
Copy link
Member

TheCakeIsNaOH commented Jan 9, 2021

The moderation page has a couple of places that could use some work.

  • The becoming a reviewer section still has is TBD.
  • The moderator review section has a note from 2015 about moderators not needing to check for duplicate packages. Is that note still valid?
  • The guidelines section has guidelines about including a #trial tag for trial software and a #license tag for software that needs a license. It says that they became requirements in Feb 2016. Is that correct? Should they be moved to the requirements section? If they are requirements, it might be a good idea to remind the moderators of the requirement because I had a package for trial software reviewed, and no comment was made that I should include a trial tag. See review comments
  • The moderator process section has a number of images that are from the old site layout. They should be evaluated if they need to be updated.
  • Add the guidance that packages should not have tags of competing products. Reference

This is more adding than fixing, but adding guidance on touching $env:ChocolateyInstall.

Where and what are packages permitted to do in $env:ChocolateyInstall?

  • Are they ever allowed to change any permissions anywhere? Probably no?
  • Are they allowed to use the tools inside $env:ChocolateyInstall\tools directly? Probably no also, but I don't think this restriction is documented.
  • Are they allowed to check what packages are installed in $env:ChocolateyInstall\lib? No, via @mkevenaar on gitter, but again, the restriction is not documented.
  • Are they allowed to directly manipulate the files in $env:ChocolateyInstall\bin? Reference gsudo install script
@gep13
Copy link
Member

gep13 commented Jan 11, 2021

@TheCakeIsNaOH thanks for raising this, and for starting the discussion on it.

Lets bring some more folks into this to make sure that they have seen this and to get their input...

@pauby @flcdrg @AdmiringWorm @mkevenaar @chtof @ferventcoder @pascalberger @mwallner @virtualex-itv

@flcdrg
Copy link
Member

flcdrg commented Jan 11, 2021

I think there are some good suggestions there. To be honest I think the 'trial' tag had slipped off my radar (as I note it was me that missed that in moderating that package). To be fair to me, I did flag that the grammar in the description was a bit odd 🙂.

The 'moderation' page is quite a long web page. I wonder if it would be better to split it out into separate pages for each section?

@TheCakeIsNaOH
Copy link
Member Author

To be fair to me, I did flag that the grammar in the description was a bit odd 🙂.

Completely, I did not word that description well at all.


One more item I have thought of: The admin tag

The powershell reference page says that the admin tag should be added if any of the administrative functions are used. However, the validator check for it was removed a couple of years ago. So is the admin tag something that should be checked for manually by moderators? Or is is completely not a requirement anymore, in which case IMO the reference to it should be removed from the PowerShell reference page.

@pascalberger
Copy link
Contributor

The 'moderation' page is quite a long web page. I wonder if it would be better to split it out into separate pages for each section?

Agreed. Splitting into different pages for how the process work and the guidelines would improve usability IMHO.

Other things which could help is to make the individual requirements and guidelines linkable, so that authors can be pointed to it. I personally also like guidelines which contain a short explanation for the why behind a specific rule (e.g. like done here for writing Cake addins: https://cakebuild.net/docs/extending/addins/best-practices#naming).

@AdmiringWorm
Copy link
Member

I think these are good things to consider.

The guidelines section has guidelines about including a #trial tag for trial software and a #license tag for software that needs a license. It says that they became requirements in Feb 2016. Is that correct? Should they be moved to the requirements section? If they are requirements, it might be a good idea to remind the moderators of the requirement because I had a package for trial software reviewed, and no comment was made that I should include a trial tag.

I know the trial tag is still a requirement that is meant to be checked by a moderator and pushed back if it is missing, not sure about the license though (mostly because I have not moderated any packages that require a license to be installed).

Add the guidance that packages should not have tags of competing products.

While I would say this should generally be true, there may be cases where such a tag is appropriate (for instance, if a competing product is part of the normal talk. Like how google is for when you mean to search for something). This is something that should be rare though.

Are they ever allowed to change any permissions anywhere? Probably no?

This is definitely something that needs to be clarified a bit better, I think it would depend on the package though.
In most cases, the only place allowed to change permissions is for the installation path of the software the package is responsible for. But that is just my opinion.

Are they allowed to use the tools inside $env:ChocolateyInstall directly?

I assume you are talking about the tools located inside the tools folder?
If so, then no these are not allowed to be used directly (same with the chocolatey binaries as well).

Are they allowed to check what packages are installed in $env:ChocolateyInstall\lib?

I don't think there is anything officially against this, but I would say it depends on the use case.
If it is only to detect if 1 of several different packages is installed it could be allowed in very rare cases.

Are they allowed to directly manipulate the files in $env:ChocolateyInstall\lib?

I assume you mean in the bin directory, not the lib directory.
Generally this is disallowed, except for very few use cases where it is absolutely necessary (we may have to look at gsudo to see if that is the case here).
Regarding the gsudo package, it looks like it is doing something it shouldn't, as it creates a symlink for the executable instead of a shim, and it removes the shim for sudo (due to the program being in conflict with gsudo it seems).
This IMO is wrong.

If you really meant the lib directory, then no it is not allowed (except for the directory of the current package).

One more item I have thought of: The admin tag

The powershell reference page says that the admin tag should be added if any of the administrative functions are used. However, the validator check for it was removed a couple of years ago. So is the admin tag something that should be checked for manually by moderators? Or is is completely not a requirement anymore, in which case IMO the reference to it should be removed from the PowerShell reference page.

The admin tag was removed from a moderators process quite a while ago (so the docs is wrong here).
All packages are assumed to require administrative privileges unless otherwise specified, or if the package is a .portable package.

@pauby
Copy link
Member

pauby commented Jan 12, 2021

Note that this is a 'duplicate' of chocolatey/choco-wiki#122 (I put the word duplicate in quotes as that repo is archived). There are also some commits highlighted there that was the start of my changes.

@TheCakeIsNaOH

Where and what are packages permitted to do in $env:ChocolateyInstall?

There is a short answer to that is that they shouldn't. Packages shouldn't change anything in that folder directly. We have spoken about this internally in the past. But I'd like to also caveat to say that there are no absolutes - each package should be judged individually.

The guidelines section has guidelines about including a #trial tag for trial software and a #license tag for software that needs a license. It says that they became requirements in Feb 2016. Is that correct?

This likely is correct, but tags are not applied in a standard consistent way nor are they really applied at all these days if we're all being honest. I tried to get an agreement on tags some time ago (I can't remember where that was, and I can't seem to find a link to it) but there wasn't any kind of agreement on it.

Add the guidance that packages should not have tags of competing products. Reference

I agree with this, but it has to be done properly. Where you are deliberately trying to skew the search results then I'd agree it needs to be removed. Where you are trying to introduce people to alternatives / similar products I'd disagree. In the reference you gave I'd agree that having amazondrive on the dropbox package is a sign of skewing the results. But having a plugin that works with both of those and therefore having those tags is not.

I think what needs to be stressed is something I've said many times before. Moderation is not a black and white process. Moderators have to be able to apply the rules individually to each package with an understanding of the implications of those rules not only to the packaging and distribution process but also to the precedents that are potentially being set. We can put many rules around this, which is what I wanted to do, but it still comes down to this being a judgement call sometimes. And it's difficult to wrap rules around that.

What maintainers need to understand is that the moderators put work into doing this on their own time. So understanding and respect is needed not only in normal comms but also that they're doing that job and sometimes they will get it wrong, sometimes maintainers won't agree with them and sometimes their click the wrong button.

I am more than happy to pick up rewriting the moderator rules again, but it requires a bit input from moderators and community which this issue has at least shown there is.

@AdmiringWorm
Copy link
Member

AdmiringWorm commented Jan 12, 2021

This likely is correct, but tags are not applied in a standard consistent way nor are they really applied at all these days if we're all being honest. I tried to get an agreement on tags some time ago (I can't remember where that was, and I can't seem to find a link to it) but there wasn't any kind of agreement on it.

@pauby here you have it: https://github.com/chocolatey/choco/issues/1363

Btw, I also found this issue (from the above issue): https://github.com/chocolatey/chocolatey.org/issues/575 on the chocolatey.org repository regarding overhauling the moderation guidelines.

@pauby
Copy link
Member

pauby commented Jan 12, 2021

@pauby here you have it: chocolatey/choco#1363

Thanks @AdmiringWorm - that is the one!

@TheCakeIsNaOH
Copy link
Member Author

@AdmiringWorm

I assume you are talking about the tools located inside the tools folder?
If so, then no these are not allowed to be used directly (same with the chocolatey binaries as well).

Correct, that was a typo on my end.

I assume you mean in the bin directory, not the lib directory.

Same as above.

Regarding the gsudo package, it looks like it is doing something it shouldn't, as it creates a symlink for the executable instead of a shim, and it removes the shim for sudo (due to the program being in conflict with gsudo it seems).
This IMO is wrong.

See this issue for why gsudo is not using a shim: gerardog/gsudo#47

The admin tag was removed from a moderators process quite a while ago (so the docs is wrong here).

👍, sounds like a pretty easy fix on the functions page then.


@pauby

I think what needs to be stressed is something I've said many times before. Moderation is not a black and white process. Moderators have to be able to apply the rules individually to each package with an understanding of the implications of those rules not only to the packaging and distribution process but also to the precedents that are potentially being set. We can put many rules around this, which is what I wanted to do, but it still comes down to this being a judgement call sometimes. And it's difficult to wrap rules around that.

So let's add note(s) next to things that are judgement calls. Something like "this is generally discouraged, but may be allowed if the package needs it according to the moderator's discretion" or "only allowed if condition x is met". Perhaps there could even be a new category alongside the requirements, guidelines, and suggestions, where there are things that moderator's look at and are decided on a case by case basis.

Some of the things I brought up are "known" things by the moderators, presumably having been discussed in the moderator chat. For the items that are on a case-by-case basis, for example, what is allowed in $env:ChocolateyInstall, what I am asking is that information also be put in the public moderation docs. I'm not asking for concrete guidelines for every possible case, I'm more asking for a heads up in the moderation docs about areas that are "touchy", and should be avoided by maintainers if possible (e.g. changing things in $env:chocolateyinstall without the use of a helper).

What maintainers need to understand is that the moderators put work into doing this on their own time. So understanding and respect is needed not only in normal comms but also that they're doing that job and sometimes they will get it wrong, sometimes maintainers won't agree with them and sometimes their click the wrong button.

Right, of course.

Putting on my maintainer hat, having things documented, even if it says that something will be looked at on a case-by-case basis, would help reduce frustration when a moderator asks for changes. For example, working on a package, submitting it, waiting four weeks for it to be reviewed, only then for a moderator to ask for changes that are not documented in the docs, and that is seemingly arbitrary (without knowing any behind the scenes conversations with other moderators), then having to fix it, and wait to get through the review queue again could be incredibly frustrating to the maintainer. I personally am aware that the moderators are doing this on their own time, and that some/many decisions are discussed with other moderators, so this is not really applicable to me specifically, but I brought this up because I think more documentation would help reduce friction in some cases.

I am more than happy to pick up rewriting the moderator rules again, but it requires a bit input from moderators and community which this issue has at least shown there is.

Let me know if there are specific things you need input on :)

@TheCakeIsNaOH
Copy link
Member Author

Yet another thing I thought of.

When/would a common functions file be suggested/required? Reference.

Perhaps add a guideline that if there is common code, it is ideal to factor it out to a separate file.

I know the coreteam repository uses the name helpers.ps1, and that seems to be a reasonable thing to standardize on for what to call the common functions file.

@pauby
Copy link
Member

pauby commented Jan 12, 2021

@AdmiringWorm said

The admin tag was removed from a moderators process quite a while ago (so the docs is wrong here).

I'm not aware of this. When was it decided and where is it documented?

@AdmiringWorm
Copy link
Member

@pauby said

I'm not aware of this. When was it decided and where is it documented?

I was sure it was talked about back in 2017 in the channel we used then, but I can't seem to find any reference there so I may be misremembering 😕.
Other than the mention by Rob here: https://github.com/chocolatey/choco/issues/1363#issuecomment-324756450 and the subsequent removal of the validator rule that @TheCakeIsNaOH linked to, I don't believe there is any documentation of it.

@AdmiringWorm
Copy link
Member

Nvm, I found a reference to it. But since it is from a private channel I will send you the link on gitter

@pauby
Copy link
Member

pauby commented Jan 12, 2021

So let's add note(s) next to things that are judgement calls. Something like "this is generally discouraged, but may be allowed if the package needs it according to the moderator's discretion"

Yeah. I think leaving it a bit more open than that but I think stating it's down to discretion is a good thing.

or "only allowed if condition x is met". Perhaps there could even be a new category alongside the requirements, guidelines, and suggestions, where there are things that moderator's look at and are decided on a case by case basis.

That's locking it down a little more but I think could be useful for some scenarios. Again, happy with that.

Some of the things I brought up are "known" things by the moderators, presumably having been discussed in the moderator chat. For the items that are on a case-by-case basis, for example, what is allowed in $env:ChocolateyInstall, what I am asking is that information also be put in the public moderation docs. I'm not asking for concrete guidelines for every possible case, I'm more asking for a heads up in the moderation docs about areas that are "touchy", and should be avoided by maintainers if possible (e.g. changing things in $env:chocolateyinstall without the use of a helper).

I know you're not asking for concrete guidelines, but others will be looking at it differently. We've seen some maintainers in the past (and it is a tiny minority) who use the documentation as something to beat moderators over the head with. So what is put in there can be either a good that it's documented, and a bad thing that it leaves holes open to people to do this. What I'm not saying is let's not do it, but more that there is thought to be given to these things.

The reason some of it is not up there is that the moderators are small in numbers so updating docs really isn't the first thing that is thought of. I've tried to take that bull by the horns in the past as I'm a bit believer of getting things down on paper but time has defeated me too.

Putting on my maintainer hat, having things documented, even if it says that something will be looked at on a case-by-case basis, would help reduce frustration when a moderator asks for changes. For example, working on a package, submitting it, waiting four weeks for it to be reviewed, only then for a moderator to ask for changes that are not documented in the docs, and that is seemingly arbitrary (without knowing any behind the scenes conversations with other moderators), then having to fix it, and wait to get through the review queue again could be incredibly frustrating to the maintainer.

I'm not sure how having 'this is on a case by case basis and at the discretion of the moderator' would remove that frustration as it's still not a statement of what needs to be done or not? Does that make sense?

I personally am aware that the moderators are doing this on their own time, and that some/many decisions are discussed with other moderators, so this is not really applicable to me specifically, but I brought this up because I think more documentation would help reduce friction in some cases.

The things I mentioned above aside, I entirely agree with you that documentation would help and in this case, the documentation is poor. I don't think any of the other moderators would disagree either.

Let me know if there are specific things you need input on :)

👍

@pauby
Copy link
Member

pauby commented Jan 12, 2021

I was sure it was talked about back in 2017 in the channel we used then, but I can't seem to find any reference there so I may be misremembering 😕.
Other than the mention by Rob here: chocolatey/choco#1363 (comment) and the subsequent removal of the validator rule that @TheCakeIsNaOH linked to, I don't believe there is any documentation of it.

It's certainly not something I ever remember being discussed. Not all package require admin rights to be installed - if you're just extracting a Zip file to a tools folder for example, or any number of .portable packages (not them all though).

Nvm, I found a reference to it. But since it is from a private channel I will send you the link on gitter

Okay, I stand corrected. But to be fair, I am old, and it was from 2018 👴

@AdmiringWorm
Copy link
Member

It's certainly not something I ever remember being discussed. Not all package require admin rights to be installed - if you're just extracting a Zip file to a tools folder for example, or any number of .portable packages (not them all though).

I do agree with you there, I think the intention was more or less to flip it, rather than having packages being marked as admin, have packages marked as a non-admin instead. Unfortunately I don't think that went anywhere, most likely due to priorities. But that isn't something that was discussed though, AFAIK (or remember).

pauby added a commit that referenced this issue Jan 12, 2021
choco-bot pushed a commit that referenced this issue Jan 12, 2021
Merge pull request #120 from TheCakeIsNaOH/remove-admin-tag

(GH-119) Remove reference to admin tag
@TheCakeIsNaOH
Copy link
Member Author

@pauby

I know you're not asking for concrete guidelines, but others will be looking at it differently. We've seen some maintainers in the past (and it is a tiny minority) who use the documentation as something to beat moderators over the head with. So what is put in there can be either a good that it's documented, and a bad thing that it leaves holes open to people to do this.

Hmm, I think I might know where some of that might come from. At the start of the moderator review section, it says:

You can only ever require a maintainer to make changes if there are findings from the requirements section.

Given the rest of the document, IMO that could be interpreted as saying that if the validator+verifier pass, and the package is ok with all of the bullet points in the required section, then the moderator is required to ok the package (possibly with suggestions for the next version).

So perhaps the wording should be tweaked, and/or a section/paragraph added that say a moderator can hold up a package if the maintainer is doing something incorrectly which is not covered in the docs.

I'm not sure how having 'this is on a case by case basis and at the discretion of the moderator' would remove that frustration as it's still not a statement of what needs to be done or not? Does that make sense?

Yes, it does.

For some of these perhaps there should be a link to gitter for people to ask ahead of time if their usage is ok. Sometimes there is already a good way to do something that they just don't know about.

Talking about this, perhaps I am actually looking for a page that has best practices for the community repository. What to do, and what not to do for community repository packages. The coreteam contributing.md has some good stuff in it for example.

@pauby
Copy link
Member

pauby commented Jan 13, 2021

@TheCakeIsNaOH

So perhaps the wording should be tweaked, and/or a section/paragraph added that say a moderator can hold up a package if the maintainer is doing something incorrectly which is not covered in the docs.

I think that's just another area that needs overhauled entirely. The Validator and Verifier can't ever check for everything and that's why the human moderator process exists.

For some of these perhaps there should be a link to gitter for people to ask ahead of time if their usage is ok. Sometimes there is already a good way to do something that they just don't know about.

I agree there should be a way to do that. We have something on Slack that would fit that purpose but we haven't really pushed that to be used. Perhaps this is the time to re-look at that.

Talking about this, perhaps I am actually looking for a page that has best practices for the community repository. What to do, and what not to do for community repository packages. The coreteam contributing.md has some good stuff in it for example.

I think that's another great idea. We have lots of resources that you could look at for this but they are not all in one place and you'd really need to know what you're looking for before you'd find it.

@TheCakeIsNaOH
Copy link
Member Author

Another possible thing to add would be the policy on archive.org wayback machine download links.

Currently, it is on a case-by-case basis, and only if the original download location is inaccessible.

Also, seems to be a consensus that adding a VERIFICATION.txt to explain why the package is using wayback machine link(s) is appropriate.
chocolatey-community/chocolatey-package-requests#845 (comment)

So, perhaps something could be added, saying that wayback machine links are on a case-by-case basis, and that a VERIFICATION.txt is required in those cases.

@flcdrg
Copy link
Member

flcdrg commented Feb 27, 2021

Regarding archive.org - I think complying with the original software license still applies. If the license didn't allow redistribution then just because someone happened to put a copy somewhere else doesn't mean we can use it. But if it is allowed, then no worries.

@TheCakeIsNaOH
Copy link
Member Author

Regarding archive.org

Are you talking about general archive.org user uploaded content, or the wayback machine, or both?

@flcdrg
Copy link
Member

flcdrg commented Feb 27, 2021

Probably not archive.org specific. Anything that has ended up on a website that isn't where it was originally hosted by the software author, and the software license did not permit redistribution

@pauby pauby self-assigned this Mar 22, 2021
@bubbasnmp
Copy link

What is the best way to get from https://community.chocolatey.org/packages/docbook-bundle/1.0.2:

There are versions of this package awaiting moderation (possibly just this one). See the Version History section below.

To see status here https://community.chocolatey.org/packages/?q=&moderatorQueue=true&moderationStatus=ready-status&prerelease=false&sortOrder=package-download-count

Can you add a blue info button next to Status with info on moderation or link to the queue?

221114_choco_github

@TheCakeIsNaOH
Copy link
Member Author

@bubbasnmp if you have a request for the community repository website, open an issue over at https://github.com/chocolatey/home

@TheCakeIsNaOH
Copy link
Member Author

I think it would be good to have sections with the procedures moderator's follow if one of the automated tests if failing and the maintainer requests help and/or an exemption.

So for the validator, that procedure might be checking if the reported problem is actually happening, and if it is, then asking internally about it, then opening an issue (or commenting on the issue if it is known), and adding an exemption.

For the verifier, that would be first seeing if it is a known reason for exemption (and documenting at least some of those like dependency requires reboot or requires some version of VS Code installed), manually testing it if possible, then adding the exemption. If it is not a known exemption reason, checking internally and exempting/pushing back to the maintainer/opening an issue as appropriate.

For the scanner, that would be the procedure if there is a high number of positives, and the procedure if there is another type of failure with it.

@bubbasnmp
Copy link

@bubbasnmp if you have a request for the community repository website, open an issue over at https://github.com/chocolatey/home

Ok. There was a comment on Reddit to put comments on this issue:

level 3
pauby
·
9 mo. ago
You're absolutely not. There is an issue on me to update these. I would encourage you, and anybody else reading this, to contribute or thumbs up comments on that issue as it will help me when re-writing the docs.

@pauby
Copy link
Member

pauby commented Dec 8, 2022

@bubbasnmp I added that message on Reddit. However, the question you asked isn't about the Moderation guidelines, it s about asking for a change to the website. For this, the advice from @TheCakeIsNaOH advice was correct.

If you are asking for changes to the moderation guidelines that this issue refers to then can you rephrase what you are asking?

@bubbasnmp
Copy link

@bubbasnmp I added that message on Reddit. However, the question you asked isn't about the Moderation guidelines, it s about asking for a change to the website. For this, the advice from @TheCakeIsNaOH advice was correct.

Long time user (sort of), first time caller - don't know enough to know where to ask questions.
I'll take the request over to the website group. Thanks!

@pauby
Copy link
Member

pauby commented Dec 8, 2022

No problem. Glad to have you here!

pauby added a commit to pauby/chocolatey-docs that referenced this issue Dec 16, 2022
As part of the Moderation documentation overhaul, I want to use Mermaid diagrams. The version of Statiq.Web we use does not support this so we need to update. This is the latest version available at the time.
pauby added a commit to pauby/chocolatey-docs that referenced this issue Dec 16, 2022
pauby added a commit to pauby/chocolatey-docs that referenced this issue Dec 16, 2022
pauby added a commit to pauby/chocolatey-docs that referenced this issue Dec 16, 2022
pauby added a commit to pauby/chocolatey-docs that referenced this issue Dec 16, 2022
TheCakeIsNaOH added a commit to TheCakeIsNaOH/docs that referenced this issue Dec 21, 2023
steviecoaster added a commit that referenced this issue Dec 21, 2023
(#119) Update iconUrl moderation information
choco-bot pushed a commit that referenced this issue Dec 21, 2023
Merge pull request #928 from TheCakeIsNaOH/raw-link-requirement

(#119) Update iconUrl moderation information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants