Skip to content

confdef: easily configure default targets - #59

Open
fenollp wants to merge 1 commit into
japaric:masterfrom
fenollp:confdef
Open

fenollp wants to merge 1 commit into
japaric:masterfrom
fenollp:confdef

Conversation

@fenollp

@fenollp fenollp commented Jan 18, 2017

Copy link
Copy Markdown
Contributor

No description provided.

@fenollp

fenollp commented Feb 11, 2017

Copy link
Copy Markdown
Contributor Author

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

@japaric

japaric commented Feb 23, 2017

Copy link
Copy Markdown
Owner

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

fenollp commented Feb 24, 2017

Copy link
Copy Markdown
Contributor Author

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

japaric commented Feb 27, 2017

Copy link
Copy Markdown
Owner

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
Copy Markdown
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