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

confdef: easily configure default targets #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fenollp
Copy link
Contributor

@fenollp fenollp commented Jan 18, 2017

No description provided.

@fenollp
Copy link
Contributor Author

fenollp commented Feb 11, 2017

@japaric What do you think about this? I think it makes it much clearer & easier to configure the base targets.

@japaric
Copy link
Owner

japaric commented Feb 23, 2017

Thanks for the PR, @fenollp.

I think it makes it much clearer & easier to configure the base targets.

The "default target" is an artifact of the current implementation. It's used to specify the TARGET of the job that runs even when the matrix (in .travis.yml) is empty. I would prefer if it didn't exist and every single target would have to be specified in the matrix. Hopefully #63 will fix that.

In summary, I'll prefer to merge #63 instead of this PR.

@fenollp
Copy link
Contributor Author

fenollp commented Feb 24, 2017

I think both PRs can work together: as you quoted, this PR is about defining base targets.
I find it time saving to just have to define the base target of each arch (osx, linux) only once.

@japaric
Copy link
Owner

japaric commented Feb 27, 2017

defining base targets.

What do base targets mean?

I find it time saving to just have to define the base target of each arch (osx, linux) only once.

That would still be the case with the othe PR (you only have to specify a target once but all targets must appear in the matrix section). OTOH, this PR adds more knobs (two env variables) for no apparent gain AFAICS.

@homunkulus
Copy link
Collaborator

☔ The latest upstream changes (presumably #63) made this pull request unmergeable. Please resolve the merge conflicts.

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.

3 participants