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

Bump build number to add session management after re-building Qt with it #7

Merged
merged 4 commits into from
Oct 24, 2016

Conversation

ccordoba12
Copy link
Contributor

Closes #4

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@ccordoba12
Copy link
Contributor Author

@ocefpaf, how can I skip Appveyor and Travis to create packages after I merge this one?

@ocefpaf
Copy link
Member

ocefpaf commented Oct 15, 2016

Carlos, only phone here so I am probably missing the context. Why do we
need to skip Win and OSX? I thought that the build worked fine using
defaults in those cases.

Either way just merge as-is and do not worry about the failures as no
package will be created anyways.

On Oct 15, 2016 7:39 PM, "Carlos Cordoba" notifications@github.com wrote:

@ocefpaf https://github.com/ocefpaf, how can I skip Appveyor and Travis
to create packages after I merge this one?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA6BL5wUd_8FRTqKDzIKmOd7SENsZ9Hpks5q0VY2gaJpZM4KX1WL
.

@@ -21,7 +21,8 @@ source:
- configure.patch

build:
number: 0
number: 1 # [linux]
number: 0 # [win or osx]
Copy link
Member

Choose a reason for hiding this comment

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

Why not just bump to 1? Seems like it should be ok to have new packages for all of them anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because nothing has changed for those OSes :-) So I wanted to reflect that we're rebuilding PyQt only on Linux.

Copy link
Contributor Author

@ccordoba12 ccordoba12 Oct 16, 2016

Choose a reason for hiding this comment

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

At least that's the standard practice we follow in Continuum :-)

Copy link
Member

Choose a reason for hiding this comment

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

@ccordoba12 that approach is fine and one can even argue that it is "more correct." We do not enforce it though b/c of the maintenance burden of following two different build numbers.

So if you want to go that route it is OK. If you do not want to do that it is also fine. As the maintainer that is your call.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer not to split the build numbers, having a build missing on the platforms that your change is irrelevant for is fine IMHO. My reason is because I like to think in terms of "build X fixed problem A" and that's easier to remember than "build X on Windows, Y on Linux and Z on OSX fixed problem A".

@ccordoba12 ccordoba12 changed the title Bump build number on Linux to add session management after building Qt with it [skip appveyor] Bump build number to add session management after re-building Qt with it Oct 24, 2016
@jakirkham
Copy link
Member

Also please re-render with conda-smithy version 1.3.3.

@ccordoba12
Copy link
Contributor Author

Ok, this is (finally) ready! :-)

Thanks to all for your patience!

@ccordoba12 ccordoba12 merged commit 20180b9 into conda-forge:master Oct 24, 2016
@ccordoba12 ccordoba12 deleted the session-manager branch October 24, 2016 18:43
@astrofrog
Copy link

@ccordoba12 - thanks!! I think this needs to be done for pyside too? (e.g. https://travis-ci.org/glue-viz/glue/jobs/169867900)

@ccordoba12
Copy link
Contributor Author

ccordoba12 commented Oct 24, 2016

Ok, right, I forgot about it :-p

@jschueller, could you bump Pyside's build number to trigger a rebuild for it? :-) Thanks!

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

6 participants