-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
CLIENT-SPECIFICATION (2.0): document removal of master branch, addition of support for individual translation archives #10148
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to fight over who has the worse English, but all wording/grammar/... changes seem debatable at best to me.
And what exactly is your rationale for going to 2.0? I kinda get that we might want to make a new client spec release with a few changes having accumulated, but why would it be a major? Because it's breaking for some clients? Some changes in the previous minors were possibly breaking too
The main motive behind the change is that the No change to date was as breaking/has a larger impact as the current one because we never removed branches before, so I think this deserves a major version bump, if other maintainers suggest a smaller bump (to v1.6) then I am open to updating it.
I want to make it clear, I am not pointing towards someone and starting a debate about whose English is better, I just made some QoL improvements which I think is right. Feel free to review it, as the client specification must be as professional as possible to the client authors, you don't need to take my word you can run the same checks through any third-party proofreading tools and you will get almost the same suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, @kbdharun! I've left some inline suggestions.
Co-authored-by: Waldir Pimenta <waldyrious@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I much prefer the new version. It's true that some changes are subjective, but it makes the document much more accessible imo. It might've been a tad too formal before.
Now was a great time to review this while dropping the master
branch anyway, and you did a good job. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this, @kbdharun — LGTM!
This comment was marked as outdated.
This comment was marked as outdated.
I don't know if this spec is the place to define the tldr pages syntax. But if so, it should be more clear that is the intent and not just an aside note. This is why I suggest restructuring the The Examples :
I've removed the notice at the beginning
I'm not sure that we need the examples if this is clear enough, you can choose whether to include them. In any case, I think that having a place where the format is clearly defined would be nice. I use |
Marking the PR as a draft, later today I plan on updating it to highlight #10555 |
Has changing the syntax for escaping highlight expressions been considered ? |
Currently no, but we will look into it (and other related issues) in the future. Offtopic: Since this PR has been open for a while now (with new changes like translation archives), I will request final reviews in the chatroom and once the green light is given, I will create a release soon. |
As promised in the chatroom, I plan to create a release informing that the
master
branch is deprecated and introduce the newer client specification after merging this PR.Note: I have pre-referenced the tag link, which will be available once it is released. The date is tentative too, we can update it later to reflect the correct date.
Changes
v2.0
, minor changes in the changelog.