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

add more precise rules to CONTRIBUTING.md #3423

Merged
merged 2 commits into from
Nov 1, 2018

Conversation

ebbit1q
Copy link
Member

@ebbit1q ebbit1q commented Oct 29, 2018

Add extra rules that clang-format enforces, like the order of includes and the way single line comments should be formatted.
Correct some grammar ( what is up with the translation section ordering steps out of order?? ).
This is a web edit.

Add extra rules that clang-format enforces, like the order of includes and the way single line comments should be formatted.
Correct some grammar ( what is up with the translation section ordering steps out of order?? ).
This is a web edit.
@tooomm
Copy link
Member

tooomm commented Oct 29, 2018

Damn, so many typos. 🙈


what is up with the translation section ordering steps out of order?

Can you give more details, please?

@ebbit1q
Copy link
Member Author

ebbit1q commented Oct 30, 2018

The translation section lists five steps but when explaining the five steps they are listed as 1, 2, 3, 5, 4; likely to separate them over different roles (dev, maintainer, translator) but it's just confusing.

@tooomm
Copy link
Member

tooomm commented Oct 30, 2018

Ahh, got you now! And you are right in your assumption, that was the idea behind it.
If we order it by numbers the process for maintainers will be split up and the reader might think he is done already without - realizing after a step not relevant for him - sometime else has to be done.

But I'll look into improving that and at least add some linking.
Maybe I've some other idea, too. Any suggestion from your side?

@ebbit1q
Copy link
Member Author

ebbit1q commented Oct 30, 2018

I say ditch the step numbers in the explanation and see them as guides per target user, it should mention in the maintainer guide that translators do something in between the last two steps of course.

@tooomm
Copy link
Member

tooomm commented Oct 30, 2018

For now I added links to the ordered structure and a jumplist per "user type":
https://github.com/Cockatrice/Cockatrice/blob/tooomm-contributing/.github/CONTRIBUTING.md#translations

Will look further and consider your input in the next days. 👍

ebbit1q added a commit to ebbit1q/Cockatrice that referenced this pull request Oct 30, 2018
Remove the weird out of order numbering and replace it with just chapters for specific users.
Add a bit of explanation to adding translations as a developer.

This is just a recommendation for an improvement, jumplist could be added and/or Cockatrice#3423 merged with this. (it fixes the typos)
@ebbit1q
Copy link
Member Author

ebbit1q commented Oct 30, 2018

I created a different pr from the web ui to show how I think the translating chapter would be more readable #3424 , feel free to close it if you want to preserve the current step layout.

ZeldaZach pushed a commit that referenced this pull request Oct 30, 2018
Remove the weird out of order numbering and replace it with just chapters for specific users.
Add a bit of explanation to adding translations as a developer.

This is just a recommendation for an improvement, jumplist could be added and/or #3423 merged with this. (it fixes the typos)
@ZeldaZach
Copy link
Member

Conflicts will need to be resolved b4 i can merge this

@ebbit1q
Copy link
Member Author

ebbit1q commented Oct 31, 2018

merged, I expected the other pr to be rejected or merged after this one.

@ZeldaZach ZeldaZach merged commit 051fcff into Cockatrice:master Nov 1, 2018
@ebbit1q ebbit1q deleted the patch-1 branch January 18, 2019 16:00
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

Successfully merging this pull request may close these issues.

None yet

4 participants