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

Handle Perl and R matrix items. #87

Merged
merged 2 commits into from
Mar 22, 2016

Conversation

pelson
Copy link
Member

@pelson pelson commented Mar 22, 2016

Again, unittests would be great, but I have tested this by hand to confirm that this now enables Perl and R support to feedstocks.

pelson added a commit that referenced this pull request Mar 22, 2016
@pelson pelson merged commit a549480 into conda-forge:master Mar 22, 2016
@pelson pelson deleted the better_matrix_determination branch March 22, 2016 08:53
@pelson
Copy link
Member Author

pelson commented Mar 22, 2016

Closes #82 by simply removing the appveyor.yml if it isn't needed.

@jakirkham
Copy link
Member

Nice, so we still register for AppVeyor, but simply don't include the CI file. Awesome fix. 👍

Just out of curiosity, under what conditions does it get added back?

@jakirkham
Copy link
Member

So, we tried removal of appveyor.yml on autoconf, but it doesn't seem to quite work because we might need to change a setting on AppVeyor. I don't know if there is a way for you to do that, as well. If not, another strategy might be to add a skip commit filter that skips all commits. This won't avoid AppVeyor jobs getting queued, but it should at least allow for a fast finish.

@pelson
Copy link
Member Author

pelson commented Mar 22, 2016

Just out of curiosity, under what conditions does it get added back?

If your recipe no longer skips the specific windows matrix item, rerendering with conda-smithy (which anybody can do) will restore the appveyor.yml.

@pelson
Copy link
Member Author

pelson commented Mar 22, 2016

So, we tried removal of appveyor.yml on autoconf, but it doesn't seem to quite work because we might need to change a setting on AppVeyor. I don't know if there is a way for you to do that, as well.

There is a toggle which states skip if no appveyor.yml in AppVeyor itself which needs to be enabled. By default it is not, and so I will have to toggle that for all feedstocks (as well as all future feedstocks).

@jakirkham
Copy link
Member

rerendering with conda-smithy (which anybody can do) will restore the appveyor.yml.

Got it. That's what I was thinking. Thanks for clarifying.

By default it is not, and so I will have to toggle that for all feedstocks (as well as all future feedstocks).

Right, what would we need to do in order to get that to work? Is it possible to automate?

@jakirkham
Copy link
Member

Are you just planning to use this API?

@pelson
Copy link
Member Author

pelson commented Mar 22, 2016

Are you just planning to use this API?

Yep

@jakirkham
Copy link
Member

So, I'm thinking about this again and I think we should just rename the CI file instead of deleting them so it is easy to get back. Mainly I am thinking about this in terms of PRs that want to add Windows support to something that didn't have it. For instance, we could move appveyor.yml to disabled_appveyor.yml. When someone wants to add support back, we just tell the to rename back to appveyor.yml. I will draw up a PR and we can discuss.

# There are no cases to build (not even a case without any special
# dependencies), so remove the appveyor.yml if it exists.
if os.path.exists(target_fname):
os.remove(target_fname)
Copy link
Member

Choose a reason for hiding this comment

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

It will probably just be changing this line honestly.

@jakirkham
Copy link
Member

Proposal to rename AppVeyor CI files here ( #94 ).

@pelson
Copy link
Member Author

pelson commented Mar 26, 2016

When someone wants to add support back, we just tell the to rename back to appveyor.yml

Not sure I agree with this statement (though I agree with your change in #94). conda-smithy shouldn't be a magical black box, and anybody should be able to:

conda install -c conda-forge conda-smithy
conda smithy rerender

There are other projects out there who demand this kind of thing from their users, and expect the rendered content to be considered a black box (e.g. versioneer).

Anyway, agree or not, the change you put forwards in #94 was a nice one 👍

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.

2 participants