nheko: 0.6.3 -> 0.6.4, mtxclient: 0.2.0 -> 0.2.1 #63515
Conversation
Use new URL of mtxclient. Make them both aware of nlohmann_json dependency using `-Dnlohmann_json_DIR` in `cmakeFlags`.
| }: | ||
|
|
||
| # These hashes and revisions are based on those from here: | ||
| # https://github.com/Nheko-Reborn/nheko/blob/v0.6.4/deps/CMakeLists.txt#L52 |
There was a problem hiding this comment.
Seems to be not relevant anymore, is it?
There was a problem hiding this comment.
So it seems that the comment is there to tell the next person looking at the package that the hashes in rev are imported from that CMakeLists file...
I'd rather put this note in the commit message, which is (IMO, tbh) the first place to look at when curious about such a thing... not in the file itself at some other place...
Maybe someone else has a different opinion though...
There was a problem hiding this comment.
Although I'm a beginner Nix user, I did understand what these lines meant - I figured these are dependencies pulled manually by the Nix build system. Therefor, I figured that I may need to update these as well according to the new version's requirement. My first attempt was to search in nheko's source tree for any mention of what specific version does it pull. At first I thought these will be git submodules versions but then I've reached that CMakeLists.txt file and found these revisions' hashes.
My opinion is that when someone will want to update Nheko to 7.0.0, hopefully he will see this comment which importantly has a v0.6.4 URL, and will check if the revisions have had an update in 7.0.0.
Additionally, not every contributor's git blame skills are good enough that he will easily find the original commit message (not mine) that added these dependencies which have hopefully (I haven't checked that myself) mentioned where did the revisions came from.
|
Why not?
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
|
Looks great, thanks! I agree with you that more comments are most often better, and the less people have to search for information the better. Here I'd maybe have a doubt about this |
Motivation for this change
Update package nheko
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nix-review --run "nix-review wip"(Non depend on it)./result/bin/)nix path-info -Sbefore and after)Other information
Nheko-RebornGitHub namspace.-Dnlohmann_json_DIRincmakeFlags.