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

Add HTTP Resolver and Support alternate valid GNUMake filenames #36

Merged
merged 7 commits into from Sep 23, 2019

Conversation

zph
Copy link
Collaborator

@zph zph commented Sep 21, 2019

(Builds off #35 but will rebase it when those are in master. So only later commits are relevant)

  1. Adds an http resolver to allow for:
include https://example.com/Makefile

I updated the readme to document this and it keeps backwards compatibility when include statement doesn't have a remote that starts with http.

  1. Allows for alternate filenames in local repo per guidelines from GNUMake, in precedence based ordering. These are:
var makefileVariants = [3]string{"GNUmakefile", "makefile", "Makefile"}

@zph
Copy link
Collaborator Author

zph commented Sep 21, 2019

The tests did not mention using a special version of tree from campoy/tools/tree, until I found it in Semaphore. That explains why my test output was succeeding but then failing after getting local to match CI (by installing campoy/tools/tree).

The tests now match the tree that is available via brew install on OSX, which is the most common implementation: http://mama.indstate.edu/users/ice/tree/.

I think it makes sense to use that one. But if you'd prefer to use the campoy version, I'll undo these changes and add it to an automatic setup step :). Either way, I should include in this PR a mechanism for getting local system to easily run tests w/ any additional tooling required.

@zph zph mentioned this pull request Sep 21, 2019
@tj
Copy link
Owner

tj commented Sep 23, 2019

ahhh yea I think it was missing in Semaphore, or maybe I was missing it, I forget now haha. I'm cool with whatever

@tj tj merged commit ad5e5de into master Sep 23, 2019
@tj
Copy link
Owner

tj commented Sep 23, 2019

LGTM!

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