proposal to close #525 by generating build/.gitignore #528
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
awvwgk
reviewed
Jul 31, 2021
awvwgk
approved these changes
Jul 31, 2021
LKedward
approved these changes
Jul 31, 2021
There was a problem hiding this comment.
Thanks @urbanjost. These changes look fine to me. If I understand correctly, this fixes a problem where the root level .gitignore can ignore valid source directories as well.
So I put the orginal .gitignore back in, and even made it bigger if "fpm new -full" is used. Here is why ... Although I preferred only auto-generating .gitignore in build/ initially, after reviewing the comments I built a list of public github projects containing fpm.toml files and pulled them and eliminated most trivial ones that appeared to just be experiments in using fpm by somewhat arbitrarily removing those with less than one hundred lines of Fortran source and found the majority of those remaining had a customized (ie. not the default one built by the "fpm new" subcommand) .gitignore file; indicating many projects need a .gitignore file in the top directory, so might as well keep seeding it(?), I found seven that did not have a .gitignore at all, reinforcing the need to put one in the build directory itself, as this still does. For the most part those appeared to be unintentional and looked like several were inadvertently storing binaries on github as a result, indicating the change to making a build/.gitignore file is a good idea. stdlib-fpm does not have one, but did not have a build/ directory on github, but the change to put one in build/ itself will prevent that being a problem for anyone that forks or clones it, so that is covered now. I found a pattern where it looks like a number of large sites are setting up to be able to build with other systems other than fpm(1), which by itself does not need a .gitignore file of any complexity; just perhaps skipping non-core files like logs and fodder and other user-specific needs it is not too easy to optimize except by exclusion (that is, exclude everything except the default files used by fpm, which does not feel like a good direction). On the other hand, I found two other others using the default github list for Fortran projects, and saw several other projects essentially making their own attempt at the same list, but I think a large .gitignore file is conflating for a new (and probably typical) user so I put back the simple one by default when "fpm new" is used, but if "fpm new -full" is used, add the github recommendations. The github recommendations do not directly benefit a pure fpm project, but at least so far that is still a minority of the sample packages I found; so adding it at all or adding it via --full is a bit kluge, but seemed like it would make handy information readily available if perhaps not in the most intuitive way, but would not conflate basic usage.
|
Thanks, John. I'll go ahead and merge this patch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Seems like #525 is reasonable; it does put a few lines that are git(1)specific into fpm, but is does not make it dependent on the .gitignore file (that is, someone not using git(1) is not affected); and is more reliable at ensuring build/ is ignored which is very dominantly the case. Any up or down votes on this simple change?