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.

Sign up for GitHub

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

Jump to bottom

Provide empty matrix on skip #479

Merged
merged 4 commits into from
Mar 14, 2017
Merged

Provide empty matrix on skip #479

merged 4 commits into from
Mar 14, 2017

Conversation

johanneskoester
Copy link
Contributor

@johanneskoester johanneskoester commented Mar 14, 2017 •
edited
Loading

This test case is able to reproduce the problems discussed in PR #453 (see here).
Further, the PR also contains the resulting fix.

@ocefpaf @jakirkham please let me know what you think.

@johanneskoester
Copy link
Contributor Author

Great, I was able to reproduce the error with the test case, see here. Commit 75dc6bf contains a possible fix.

@ocefpaf
Copy link
Member

ocefpaf commented Mar 14, 2017

Awesome @johanneskoester! Thanks for tackling this!!

ocefpaf
ocefpaf reviewed Mar 14, 2017
View reviewed changes
conda_smithy/configure_feedstock.py
@@ -603,6 +603,10 @@ def meta_of_feedstock(forge_dir, config=None):


def compute_build_matrix(meta, existing_matrix=None, channel_sources=tuple()):
if meta.skip():
Copy link
Member

Choose a reason for hiding this comment

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

Seems sensible. Thanks for looking into this.

@johanneskoester
Remove test case for linux-32.
8b050af 
It is not common to skip in in conda-forge and we don't build against it.
Hence, the provided error message is correct.
@johanneskoester
Define channel sources.
84af9a7
@johanneskoester
Copy link
Contributor Author

Ok, I consider the issue to be fixed with this PR. If you agree, can we have a quick release? @pelson @jakirkham @ocefpaf.

@johanneskoester johanneskoester changed the title Add test case for configuring r related feedstocks. Add test case for configuring r related feedstocks. Fix compute_build_matrix in combination with build->skip. Mar 14, 2017
@ocefpaf
Copy link
Member

ocefpaf commented Mar 14, 2017

LGTM

@ocefpaf ocefpaf merged commit afd233e into conda-forge:master Mar 14, 2017
@ocefpaf ocefpaf mentioned this pull request Mar 14, 2017
Merged
@jakirkham
Copy link
Member

Thanks @johanneskoester. A good fix indeed.

@johanneskoester
Copy link
Contributor Author

Will there be a release, such that I can rely on it with upcoming R pull requests?

@jakirkham
Copy link
Member

I wanted to sort out the other R issues. ( #447 ) Otherwise we will just have different problems with R packages.

Also wanted to try and consolidate the team update code in conda-smithy ( #466 ). So that we can setup something to manage team updates. Right now the current strategy is totally broken.

I don't know that I will have time to do all of that today. Maybe near week's end.

@jakirkham jakirkham modified the milestones: 2.3.0, 2.1.0, 2.2.0 Mar 14, 2017
@johanneskoester
Copy link
Contributor Author

Ok, that sounds great and makes a lot of sense. Thank you!

@jakirkham jakirkham changed the title Add test case for configuring r related feedstocks. Fix compute_build_matrix in combination with build->skip. Provide empty matrix on skip Mar 19, 2017
@jakirkham jakirkham mentioned this pull request Mar 21, 2017
Closed
@ocefpaf ocefpaf mentioned this pull request Mar 21, 2017
Merged
@johanneskoester johanneskoester mentioned this pull request Mar 23, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ocefpaf ocefpaf ocefpaf left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@johanneskoester @ocefpaf @jakirkham