Closed fastrgv closed 3 years ago
I've been looking at your code but can't see the snd4ada files anywhere on Git. From my point of view, I think that even if the code is in the distributed archive, you should make the Git project clearer before linking it to the Awesome Ada collection.
Dear fastrgv,
First, thank you for your interest in the awesome-ada list.
Upon closer inspection of your PR, I decided to not merge it, as-is, for the following reasons:
1) Archive distribution (.7z) should only contain binaries, not the source code we need to inspect and/or build and/or try your repository. It goes against the fundamental purpose of a source control management tool like Git. Eg. In your case the example Ada files and build scripts are in the .7z archive; they should be directly under Git, distributed as text source. A better practice would be to distribute the binaries either as Github releases or use a git submodule configured with git-lfs (git-lfs can be omitted if you do not want to version the binaries and they are small).
2) A quick look at your windows build script shows you encoded a hard path for a GNAT distribution. Eg. on my system, this would fail to build as I have my GNAT installed somewhere else entirely. Recommendation: Use gprbuild to configure the build or, if you want to keep a custom build arrangement, make sure your build/install instructions are more flexible/detailed so we can properly configure your build tools against the specifics of our machine. Hard encoded paths should only be relative and within your source distribution, else they cannot be relied upon.
3) The distinction between original source code, 3rd party resources, and data is very unclear. If I may, your code distribution could be much clearer if arranged like so:
ada_open_al.gpr
.gitignore
readme.md
data/
choir.wav
...
source/
snd4ada.ads
snd4ada.adb
...
examples/
one.ads
one.adb
...
binaries/ (maybe as a git submodule so you could share it/include it in your separate C++ repo)
openAL32.dll
...
4) The outputted GNAT build files found in the obj/ folder should not be distributed under Git. To do so, please add an entry obj/
to a .gitignore file added to your repo.
5) Having a floating C++ version alongside is really a hard sell for the inclusion into an awesome-ada list. Is it an Ada repo or a C++ repo? It could be argued that such a C++ version should have its own Git repository.
I am closing this pull request for the moment. Please address those comments by amending your repository and reopen or resubmit a pull request to be added to this awesome-ada list. I will gladly review the changes and include your work given it is up to standards.
Sincerely,
Olivier
You may also want to read this blog post on how to write good commit messages, which helps the maintainability of your project.
I believe I have now addressed all your concerns. Please take a look. Rod aka fastrgv@gmail.com
On Tue, Feb 16, 2021 at 11:06 AM onox notifications@github.com wrote:
You may also want to read this blog post https://chris.beams.io/posts/git-commit/ on how to write good commit messages, which helps the maintainability of your project.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ohenley/awesome-ada/pull/66#issuecomment-780053895, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRD7YODXTLKWY2FKI6VSFLS7K62JANCNFSM4XSGPOUQ .
please add