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

Explicitly include test file in STRTA #26

Merged
merged 1 commit into from
Oct 28, 2020

Conversation

colekettler
Copy link
Contributor

Overview

After templating a new project, I noticed that scripts/test was missing from the scaffolded project. Turns out, g8 ignores files named test by default! A full list of defaults is available here.

This was addressed with the introduction of the .g8ignore file, which declares files to be excluded from the templated project. By declaring a negation rule for scripts/test, we override the default and include the file.

This regression was introduced in #10.

Testing Instructions

  • Run sbt new azavea/azavea.g8 and accept the defaults.
  • Run ls quickstart/scripts. test will be missing.
  • Remove the quickstart project with rm -rf quickstart.
  • From this PR branch, recreate the scaffold with sbt new file:///path/to/local/azavea.g8.
  • Run ls quickstart/scripts. test should be present.

@colekettler colekettler self-assigned this Oct 28, 2020
Copy link
Contributor

@jisantuc jisantuc left a comment

Choose a reason for hiding this comment

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

I wonder if the ubuntu-latest github actions environment has tree. If so it might be nice to add tree-ing the created project to CI, not for specific expectations, but because (at least right now) the sample project is 58 lines of tree and if the output looks halfway normal testing instructions for file inclusion / exclusion things like this could be "check the logs" 🤔

@@ -0,0 +1,3 @@
# Files named `test` are ignored by default, this explicitly includes it.
# See https://github.com/foundweekends/giter8/issues/221
!scripts/test
Copy link
Contributor

Choose a reason for hiding this comment

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

🆒

@colekettler
Copy link
Contributor Author

@jisantuc Unfortunately tree is not included! Tested it in this run using which tree. Installing it would be pretty easy though, and I agree that it could be useful for debugging purposes. I've set up #28 to track it, and I'll probably end up pulling it later today.

@colekettler colekettler merged commit 431b227 into master Oct 28, 2020
@colekettler colekettler deleted the feature/cek/fix-ignored-test-file branch October 28, 2020 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants