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

readme: fix tilde character display #2582

Merged
merged 1 commit into from
Jan 14, 2021
Merged

readme: fix tilde character display #2582

merged 1 commit into from
Jan 14, 2021

Conversation

bl-ue
Copy link
Contributor

@bl-ue bl-ue commented Jan 11, 2021

  • Fixes display of ~ character in GitHub in compiler table (it used to look like this: ...1exp1... instead of ...1~exp~1...)

@bl-ue bl-ue requested a review from nlohmann as a code owner January 11, 2021 18:39
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 7dda51b on bl-ue:patch-1 into 14be8c6 on nlohmann:develop.

@YarikTH
Copy link
Contributor

YarikTH commented Jan 12, 2021

I'm not a maintainer, but here is my IMO. This PR should be divided into "readme: harmonize versions" and "readme: fix ~ char display".

While ~ display fix is a really useful fix, version harmonization is discussible.

Nobody wants to maintain this manual version harmonization, so version numbers in readme will be outdated immediately after another release. I think, that all such mentions should be universal one like merely add `nlohmann_json/x.y.z` to your `conanfile`'s requires, where `x.y.z` is the release version you want to use. or makefile should contain some new step for updating version in the readme.md (using md commentary as a mark?) and such make step shouldn't be forgotten somehow while releasing. Otherwise, I fear it's useless work.

@bl-ue
Copy link
Contributor Author

bl-ue commented Jan 12, 2021

Okay @YarikTH. I thought that @nlohmann could update versions in the README just the same way he does in the rest of the project although I haven't actually checked to if it's automated or not (just a global find/replace?)

@YarikTH
Copy link
Contributor

YarikTH commented Jan 12, 2021

I thought that @nlohmann could update versions in the README just the same way he does in the rest of the project although I haven't actually checked to if it's automated or not (just a global find/replace?)

I have no idea, but the fact, that version numbers in readme are outdated, means that there is either no such automatization step, or it is not executed for a very long time. So maybe it's time to invent one and integrate into the release process.

@bl-ue
Copy link
Contributor Author

bl-ue commented Jan 12, 2021

Good idea. I'll do some research and report back.

@bl-ue
Copy link
Contributor Author

bl-ue commented Jan 12, 2021

@nlohmann is the version updated as in commit b3e5cb7 through automation of any sort?

@YarikTH
Copy link
Contributor

YarikTH commented Jan 12, 2021

just a global find/replace?

It's a terrible idea.

Because at least readme contains a lot of version numbers of another software. For e.g. clang

Clang 3.5.0 (3.5.0-4ubuntu2~trusty2) | Ubuntu 14.04.5 LTS | Travis
Clang 3.6.2 (3.6.2-svn240577-1~exp1) | Ubuntu 14.04.5 LTS | Travis
Clang 3.7.1 (3.7.1-svn253571-1~exp1) | Ubuntu 14.04.5 LTS | Travis
Clang 3.8.0 (3.8.0-2ubuntu3~trusty5) | Ubuntu 14.04.5 LTS | Travis
Clang 3.9.1 (3.9.1-4ubuntu3~14.04.3) | Ubuntu 14.04.5 LTS | Travis
Clang 4.0.1 (4.0.1-svn305264-1~exp1) | Ubuntu 14.04.5 LTS | Travis

There is even a full match with the current release version. So all the versions of all the mentioned software would become "the same release" as nlohmann/json. Like a rolling snowball.

@bl-ue
Copy link
Contributor Author

bl-ue commented Jan 12, 2021

That wouldn't be good, but then, how does he do it currently? 🤔

@nlohmann
Copy link
Owner

Version numbers are updated in a script I run when I prepare a release. The version numbers of the README file are usually not part of it. The versions for the package managers are just examples - I am not a maintainer for those and I'm not willing to check regularly which version is available where.

So I concur with #2582 (comment) and would like to see a PR handling the tildes, but rather not touch the version numbers in the README file.

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not adjust the version numbers, see #2582 (comment).

@bl-ue bl-ue changed the title readme: harmonize versions; fix ~ char display readme: fix ~ char display Jan 14, 2021
@bl-ue bl-ue changed the title readme: fix ~ char display readme: fix tilde character display Jan 14, 2021
@bl-ue bl-ue requested a review from nlohmann January 14, 2021 21:02
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann self-assigned this Jan 14, 2021
@nlohmann nlohmann added this to the Release 3.9.2 milestone Jan 14, 2021
@nlohmann nlohmann merged commit 92fa1d9 into nlohmann:develop Jan 14, 2021
@nlohmann
Copy link
Owner

Thanks!

@bl-ue bl-ue deleted the patch-1 branch January 14, 2021 21:15
@YarikTH YarikTH mentioned this pull request Mar 18, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants