Hi Michal, thanks for your thorough feedback! More comments inline.
On Thu, Apr 29, 2021 at 5:14 PM Michal Schorm mschorm@redhat.com wrote:
Hello,
The "Contributing to CentOS Stream" webpage: https://wiki.centos.org/Contribute/CentOSStream is very confusing (at least to me) and lacks a lot of details.
It's true the documentation is not finished yet - the CentOS Stream team is planning to provide comprehensive contributor's documentation, the wiki is really just an intro.
Here is my proposal to update / fix / enhance / clarify at least some of the sections.
--
(1) Section 1 Text: "You should always open a bugzilla first"
The link leads to the most general selection in Bugzilla. Even in the "All" category, there is no "CentOS" or "CentOS Stream" product. Let's clarify that the product selected should be "Red Hat Enterprise Linux *" and provide more accurate link: https://bugzilla.redhat.com/enter_bug.cgi?classification=Red%20Hat
Oh yeah, this is gonna be really handy! I just updated it to point to RHEL 8 and picked CentOS Stream as the version.
(2) Section 1 Text: "You should always open a bugzilla first"
"Should" ? The process *requires* package maintainers to have the ACKed BZ referenced in the commit message, otherwise it will be impossible to merge it. I haven't heard of any other way, so this should be changed to " *MUST* ".
The reason I wrote "should" there is that it may already be a bugzilla opened for the MR. I can see that Rich already changed it to "must" so let's keep it that way because that would incentivize potential contributors to discuss changes first, which is what we really want.
Also, the GitLab offers features such as "Issues" and so on. Will those be globally disabled to discourage the contributors to file tickets in the wrong place ?
Yes, issues are disabled. Bugzilla is still the official tracker.
(3) Section 1 Text: "As with any open source projects, in the end it's up to maintainers to decide which changes they want to accept and maintain, so Red Hat maintainers can decline contributions for any reason."
This sentence is heavily suggesting that only "Red Hat maintainers" have the power to decide the fate of the contributions.
Correct.
Does that mean that there can be no CentOS stream maintainer that is not a RH employee at the same time ? If yes, why don't make this clear and say it explicitly ?
I'd say it's explicitly stated there already. Please suggest a better wording if you have one in your mind already.
Also if yes, why can't I login to CentOS project using Red Hat credentials or SSO ?
Because the CentOS identity provider is not utilizing Red Hat's SSO - I'd suggest starting a new thread for such a request because that's a big one. Though you can log in to GitLab via https://red.ht/GitLabSSO
If not, the sentence doesn't make much sense.
(4) Section 1 Text: "It's also great to have it in Fedora Linux first as well."
AFAIK, the OS you are mentioning is named "Fedora" or you can use "Fedora Operating System" (to distinguish from the Fedora Project that has a broader portfolio than just the OS). I think the "Fedora Linux" is not correct. Correct me if I'm wrong
https://communityblog.fedoraproject.org/fedora-is-a-community-fedora-linux-i...
Also, shouldn't be the "It's also great to have it in ..." statement which is "nice-to-have" like, rather something more stressing the fact that Fedora serves as an upstream for RHEL an in order to avoid future regressions, it is very important to have the change accepted in the Fedora first - if possible && if it makes sense. The Red Hat package maintainers have to do it (the upstreaming of relevant code to Fedora) anyway as a part of their job. Also, how would it need to be a part of the upstream project and not to be in Fedora at the same time ? I mean Fedora (at least Rawhide) aims to provide the latest upstream versions possible.
This is really hard because you have plenty of software that is being built and configured differently for all the distributions: kernel, qemu-kvm, *-release, subscription-manager...
I'll reword that statement and will make it more clear. My assumption is there would be a dialogue between the maintainer and the contributor on how to handle specific contributions: I'd say it's perfectly reasonable to ask the contributor to propose the patch upstream or to Fedora Linux first.
(5) Section 2 Text: "If you want to contribute to CentOS Stream, you need to have an account in CentOS FAS."
Same issue as (3), same questions I ask.
How comes it that I do not have the account and still I can contribute ? The statement is most likely false.
I agree this piece is slightly outdated because the sources are now placed on two locations: git.centos.org and gitlab.com/redhat/centos-stream
I'll update it.
(6) Section 3 Text: "Once your account is set up, you can proceed to clone the respective package repository locally."
You can't clone the package repository locally without any account at all? That would be weird for a public Git repos ...
I'd reformulate the sentence such as "The next step you like to do is to clone ... " Also, the actual goal is not to clone the repo, but fork it, so let's state that fact right away: "The next step you like to do is to clone a package (project) from one of the following repositories and fork it"
That's a good point, I simplified the section.
(7) Section 3 Text: "The package repositories are right now located in two places:"
Two places, but four list items? :)
will update
(8) Section 3.1 Text: "You'll be able to see 3 types of branches:
Not all projects have all 3 types of the branches. It would be better to say something like "The repository will contain some of these types of branches:'' followed by a bullet point list
" To me, it already says that. Would this wording work? "Branches in the repositories follow this naming pattern:"
(9) Section 3.1
Also, what about showing some instructions on how to do it? Do the contributors from the general public know about the "centpkg" tool or should they work with bare "git" ?
I'll mention centpkg though it doesn't have comprehensive documentation out there yet.
If they should know (or even are encouraged to use) "centpkg", provide the damn instructions.
I don't think such language is necessary.
Otherwise, describe if the forking in the web UI is the only way and how to do it.
(10) Section 4.1
I bet this is a confusing section, at least for new contributors.
1/ Let's begin with the fact that you need the fork in the first place. 2/ Inside of the fork, you should create a new branch - based on the branch they want to contribute to, ideally (but necessarily? ... we might make a "best practice" though) named "bz12345" referencing the submitted BZ ticket for easy recognition 3/ commit the changes. note: each commit must contain an ACKed BZ ticket in order to be able to be merged to the target branch. note: there will be different ACK requirements for different branches in the future (e.g. Y-stream v s Z-stream) 4/ Push the branch to your fork 5/ create MR note: when using "centpkg" as I asked above, the MR URL will be created during the push
(11) Section 4.2 Text: "you need to execute %prep phase to see it."
To "see it"? More like to "retrieve it from the cache".
(12) Section 4.4
What the hell is this about? We should be talking about a dist-git layout & workflow in this section, not a RPMbuild layout (which is completely irrelevant here). There are no "SOURCES" and "SPECS" directories in dist-git. that is completely wrong, the whole section.
There is: https://git.centos.org/rpms/systemd/tree/c8s
The main problem with this wiki page is that it was written as contributor's docs for CentOS Stream 8 while it was hosted on git.centos.org and it was never fully updated for Stream 9 and gitlab, hence the confusion. And also as you can see, Stream 8 and Stream 9 dist-git repos look differently.
This is where I'm stopping my replies because your language is just way too offensive and I am not interested in this kind of discussion.
1/ If you want to make just a packaging change, you *likely* only need to modify just the SPECfile. 2/ If you want to make justa patch change, you need to add the patch AND modify the SPECfile too.
At the end, you add all the modified files, commit them, add the BZ ticket to the commit message and create MR.
(13) Section 4.5 Text: "4.5. After a pull request is created"
For a reason I don't know, some people call it "pull requests" (e.g. in Fedora), some people "merge requests" (e.g. in GitLab). Anyway, we are working with GitLab, so use it's naming.
So: "4.5. After a merge request is created"
Please find all 3 occurrences of the word "pull" on the page and fix them.
(14) Section 4.5 Text: "When you submit a pull request for source-git, automation will pick up the change and build it "
Does it not work for the dist-git too? Why ? What about mentioning the scratch-build then? ... aww hell, we are back to the "centpkg" again :)
(15) Section 4.5 Text: "Once your change is accepted, it will be marked with the label "accepted". Our automation will create a corresponding bugzilla with the patch attached and then it’s up to the RHEL maintainer to pick it up, commit and build internally so that it can land back into CentOS Stream"
This is most confusing.
Why won't a maintainer merge it into CentOS first? Why would he first apply it in the most downstream place? What about the "upstream first" rule?
How can a package maintainer mark the MR as "accepted"? I haven't found such an option anywhere.
"Our automation will create a corresponding bugzilla with the patch attached" So ... another BZ ? This time to ... what product ? Again see (1).
I personally actually created a MR, but none BZ has been created: https://gitlab.com/redhat/centos-stream/rpms/mariadb-connector-c/-/merge_req...
"RHEL maintainer to pick it up, commit and build internally so that it can land back into CentOS Stream" That sentence suggests that the RHEL is the upstream for CentOS stream, but at the same time we are pushing CentOS stream to be upstream for RHEL. Get this straight.
BTW on a webpage: https://wiki.centos.org/Contribute#CentOS_Stream the re is a clear statement: "CentOS Stream is a continuously delivered distribution which is upstream to the next minor version of Red Hat Enterprise Linux"
(16) Section 5 Text: "it needs to be reviewed and approved by the RHEL maintainer for the particular component."
Again - so only RH employees will be able to maintain ? Why don't have a CentOS FAS access already created ? Why would anyone else need a CentOS FAS ?
"maintainer for the particular component" Also, I believe this is a false statement too. The current permissions AFAIK allow for any RH employee with the GitLab account with the "devel" role over gitlab.com/redhat space to merge the MR.
(17) Section 5 Text: "We are making sure behind the scenes they see your patch."
Who is making sure? the RH employees ? Who are "they" *the same* RH employees ? :D
(18) Section 6
Might help to add instructions on how to do it.
(101)
Is there not a single documentarist as a part of the CentOS project who would read through the webpages ?
--
Michal Schorm Software Engineer Core Services - Databases Team Red Hat
--