Skip to content

Conversation

@SylvainCorlay
Copy link
Member

@SylvainCorlay SylvainCorlay commented Mar 20, 2020

Fixes #194

I would really advocate from completely removing the brand_python.py though. I don't think that modifying _sys_version is a clean way of changing the banner.

Checklist

  • Used a fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@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.

I do have some suggestions for making it better though...

For recipe:

  • License is not an SPDX identifier (or Other) nor an SPDX license expression.

@isuruf
Copy link
Member

isuruf commented Mar 20, 2020

@conda-forge-admin, rerender

@SylvainCorlay
Copy link
Member Author

Even if this works, I think we should consider completely removing that patch.

@jjhelmus
Copy link
Contributor

I'm fine removing this patch entirely. It was in conda-forge before I started working on the python recipe so I kept it.

@mingwandroid
Copy link
Contributor

What about using the anaconda branding mechanism instead? We change the c code. I can template the patch and apply the correct replacement at build time if that's acceptable?

Branding is kinda useful if you change things significantly and I think, for better or worse we do that. But I don't mind if we remove it entirely for conda-forge either really.

@jakirkham
Copy link
Member

Having some kind of branding is useful I think. Though I don't know if we are discussing removing it or just some minor tweaks at this point.

@SylvainCorlay
Copy link
Member Author

What about using the anaconda branding mechanism instead? We change the c code. I can template the patch and apply the correct replacement at build time if that's acceptable?

I don't know where to find that recipe?

@SylvainCorlay
Copy link
Member Author

Having some kind of branding is useful I think. Though I don't know if we are discussing removing it or just some minor tweaks at this point.

This PR does a minor tweak but I was suggesting we should consider removing it completely.

@SylvainCorlay
Copy link
Member Author

For your information, I opened #332 completely removing the patching of sys.version - and which I think preferable over this approach.

@SylvainCorlay
Copy link
Member Author

Any interest in getting this in?

@isuruf
Copy link
Member

isuruf commented Mar 23, 2020

Can you bump the build number?

@SylvainCorlay
Copy link
Member Author

Can you bump the build number?

Done

@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.

I do have some suggestions for making it better though...

For recipe:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

@SylvainCorlay
Copy link
Member Author

@isuruf I bumped the build number - and did some more testing locally.

@SylvainCorlay
Copy link
Member Author

The rerender seems to be problematic. I don't see why there should be a linux_python3.7 build of Python 3.8...

@isuruf
Copy link
Member

isuruf commented Mar 23, 2020

One build in linux-64 passing is enough

@isuruf isuruf merged commit 8da5111 into conda-forge:master Mar 23, 2020
@SylvainCorlay SylvainCorlay deleted the brand-regex-fix branch March 23, 2020 17:24
@SylvainCorlay SylvainCorlay mentioned this pull request Apr 6, 2020
5 tasks
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.

6 participants