Questions about PR requirements and the contribute docs

Contributors
1

Am I correct in saying this about a PR:

  1. Contributors should not be creating PRs from their master branch.

Do we enforce this or is it just a guideline to help the user not soil their master branch? The contribute docs seem a little vague on this I feel. First, it shows up as more of a suggestion, then later it seems more like a requirement. Is it a suggestion, or a requirement? Do we accept PRs made from the contributor’s master branch?

  1. The PR should only make changes to the English language files.

I seem to remember this rule being part of the contribute docs but I can’t find it in the new docs. Did we remove it/never add it in the new docs, or am I just blind?

  1. If the PR makes changes to multiple files it should be in one commit, if it is not, it should be squashed.

I can’t find this information in the new docs, but I believe this information was in the old docs?

2

the checklist you have to check before submitting says that the PR should make changes to only one language
I think it is to keep a page in other languages coherent with themselves, we don’t want to have the text say something and the code say something else

1 Like
3

I know that. I’m just wondering if we forgot to add it to the docs or if it was intentionally removed.

4

It is a suggestion, and we do accept PRs from contributors’ master branches.

I have come across situations where having multiple commits to a PR made it easier to edit. In general, it might be better for the PR to be made of one commit, if one of those files needs to be removed from the commit, it is easier to deal with if it was committed seperately.

5

This is a highly recommended suggestion. But, in most cases is a suggestion like any other. Sometimes we end up closing/rejecting PRs because the master on the fork was soiled beyond recovery.

That’s when it gets ‘enforced’ sort of.

6

We expect changes to one language at a time in most cases. This is for the ability to QA them independently and not wait for other language contributors.

This is really at the discretion of the mods. For example, if we are replacing links in one challenge, it may be fine to switch them in all languages because it does not matter.

Sometimes it may not be the case when the verbiage is being changed.

What would happen is something in the english language version of the PR would wait for a long while.

7

This can be removed, I think. We are squashing PRs by default. But please note this is a source of pain for maintainers when we have 100s of files on one PR spread across commits.

8

@raisedadead Thank you for the reply.

  1. So I should just repeat the recommendation of making a new branch in the PR, but just not require it as such?

  2. What if the changes are to the challenge tests?

  3. I see, I was just wondering if the information had been forgotten or removed intentionally.

Here is an example PR.

I was going to suggest they read the Contributing documentation, recommend they use a new branch and tell them to read the PR checklist about changes to other language files. Just trying to get this right.

9

Thanks for sharing the PR.

I would accept the PR after testing is locally (or on GitPod).

I would also point them that their copy of the master branch is now soiled, they would need to sync it up correctly once the PR is merged (important). I would then point them to syncing instructions in the guide.

We are happy to accept improvements to the docs obviously if that would help.

10

Sorry for all the questions, but I might as well ask them now.

I’m confused about how are we are keeping the different language files in sync and if they need to match or what?

For example:

  1. In the PR I linked to I can see the English and Russian versions has the solution code but it is missing from the other languages.

  2. I can see in some of the language files the extra assert message at the end is still there which was cleaned from the others.

  3. The Russian language file does not have the assert inside a string but the rest of the language files do.

Do we mainly just care that all the tests are passing in the English version or how does that work? How are the other files checked? Do we run the test command against them?

11

We are currently following what is called as ‘eventually consistent’ data. That is its very difficult to keep everything in sync, but the goal is to make it as consistent as possible.

This workflow id hopefully going to be more clear once we have some tools in place to make it explicit in PRs. We are building tools to help mods get insights on what’s changed and if it needs updating in other places.

There are some PRs in the works too.

1 Like