| Message ID | 0e2c18a84e232f73a72d755a692feaece0118396.camel@student.tugraz.at |
|---|---|
| Headers | show |
Hello, Leo Prikler <leo.prikler@student.tugraz.at> writes: > Following the release of version 1.10, I've updated my package > descriptions. I've also been able to unbundle some sources, but not > all of them. OOC, did you try to symlink sources instead of copying them? Note that you don't mention what is left to unbundle. Would it make sense to add this information? Otherwise, LGTM, barring the description of spirv-cross, where I suggest to drop the "NOTE:" part. It reminds me taisei package felt through the cracks. I'll apply it once spirv-cross is pushed. Regards,
Hello, Am Dienstag, den 30.06.2020, 00:02 +0200 schrieb Nicolas Goaziou: > Hello, > > Leo Prikler <leo.prikler@student.tugraz.at> writes: > > > Following the release of version 1.10, I've updated my package > > descriptions. I've also been able to unbundle some sources, but > > not > > all of them. > > OOC, did you try to symlink sources instead of copying them? No, but I'd imagine it would not work out great, because the source is a git checkout in /gnu/store, which is read-only and it is compiled in- place. > Note that you don't mention what is left to unbundle. Would it make > sense to add this information? I'm looking mostly at the contents of ext/ here, which has stuff we already package, like cityhash among others. A bit of context is given in the unbundling snippet I use for glslang and spirv-cross, which I've managed to unbundle, but the rest are not pulled in as submodules, so it would require a `guix build --source` and manual analysis to sift through all that. I don't think a less vague comment could improve this understanding without risking to be invalidated at some point in the future. Perhaps I could clarify, that it's "mostly ext/", but even that is not completely true, since upstream also has ffmpeg as a toplevel submodule (which I don't pull in, instead using the flags they already provide for using system ffmpeg). > Otherwise, LGTM, barring the description of spirv-cross, where I > suggest > to drop the "NOTE:" part. It reminds me taisei package felt through > the > cracks. I'll apply it once spirv-cross is pushed. To be fair, that's where I copied it from – I think I already mentioned that. I just updated the version and added some details in the comment regarding failing tests and hence added my updated patch. I also think this description to be quite weird, but at the same time am unsure on how to better rephrase it. I will send an updated version of it once I do come up with something better than just dropping the "NOTE:" part. Regards, Leo
Hello, Leo Prikler <leo.prikler@student.tugraz.at> writes: > No, but I'd imagine it would not work out great, because the source is > a git checkout in /gnu/store, which is read-only and it is compiled in- > place. Oh ok. I didn't get it was compiled in-place. > I'm looking mostly at the contents of ext/ here, which has stuff we > already package, like cityhash among others. A bit of context is given > in the unbundling snippet I use for glslang and spirv-cross, which I've > managed to unbundle, but the rest are not pulled in as submodules, so > it would require a `guix build --source` and manual analysis to sift > through all that. > I don't think a less vague comment could improve this understanding > without risking to be invalidated at some point in the future. Perhaps > I could clarify, that it's "mostly ext/", but even that is not > completely true, since upstream also has ffmpeg as a toplevel submodule > (which I don't pull in, instead using the flags they already provide > for using system ffmpeg). Fair enough. >> Otherwise, LGTM, barring the description of spirv-cross, where I >> suggest >> to drop the "NOTE:" part. It reminds me taisei package felt through >> the >> cracks. I'll apply it once spirv-cross is pushed. > To be fair, that's where I copied it from – I think I already mentioned > that. I know. And this is what reminded about taisei. > I just updated the version and added some details in the comment > regarding failing tests and hence added my updated patch. I also think > this description to be quite weird, but at the same time am unsure on > how to better rephrase it. I will send an updated version of it once I > do come up with something better than just dropping the "NOTE:" part. Well, I still think dropping the note is appropriate, but I won't fight over it. Let me know if you find something that suits you better. I applied your patches. I tweaked comments. In particular comments starting with two semicolons are full sentences and are supposed to start with a capital and end with a full stop. Thank you! Regards,