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

Rebuild for hurry.filesize 0.9 is not supported on this platform #1

Merged
merged 5 commits into from
Aug 21, 2024

Conversation

rxm7706
Copy link
Contributor

@rxm7706 rxm7706 commented Aug 18, 2024

Checklist

  • Used a personal 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/django-handyhelpers-feedstock#18

import: 'handyhelpers'
+ pip check
hurry.filesize 0.9 is not supported on this platform
WARNING: Tests failed for django-handyhelpers-0.3.27-pyhd8ed1ab_0.conda - moving package to /home/conda/feedstock_root/build_artifacts/broken

@rxm7706 rxm7706 requested a review from raivivek as a code owner August 18, 2024 03:13
@conda-forge-webservices
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/meta.yaml) and found it was in an excellent condition.

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

For recipe/meta.yaml:

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

Documentation on acceptable licenses can be found here.

@rxm7706
Copy link
Contributor Author

rxm7706 commented Aug 18, 2024

@conda-forge-admin, please rerender

@conda-forge-webservices
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/meta.yaml) and found it was in an excellent condition.

@rxm7706
Copy link
Contributor Author

rxm7706 commented Aug 18, 2024

@conda-forge-admin, please rerender

@rxm7706
Copy link
Contributor Author

rxm7706 commented Aug 18, 2024

@raivivek - could you review and merge please.
Thanks and regards.

@iamthebot
Copy link

Ah, I see you ran into the same issue (I made #2 which I realize you got to first). @rxm7706 mind also adding me as a maintainer? We depend on this package heavily too and familiar w/ maintainer duties.

@rxm7706
Copy link
Contributor Author

rxm7706 commented Aug 21, 2024

@conda-forge-admin, please rerender

@rxm7706
Copy link
Contributor Author

rxm7706 commented Aug 21, 2024

@conda-forge/core - Can I be added as a maintainer - I am happy to help keep this feedstock updated.
This is required for feedstocks I am trying to upgrade and maintain.

Recipe has been refreshed and this PR (Description), will Rebuild for PiP Check Error
Thank you and regards

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

Normally we ask to wait for 1-2 weeks to give the maintainers time to respond, but based on the GH profile of the sole maintainer here, they haven't had any activity in conda-forge for at least a year (last blip was some bioconda stuff in October 2023), so I don't think there's a big chance for a response either way.

name: hurry.filesize
name: {{ name|lower }}
Copy link
Member

Choose a reason for hiding this comment

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

As an aside, this is a pessimization in my eyes. Yes, the bot autogenerates this format, but it gives the false impression that the name is a variable that could be changed, and has zero benefits, only downsides (e.g. it makes it harder to copy-paste the correct URL). If any thing, you should be replacing {{ name|lower }} when you encounter one in a recipe, but at the very least please don't introduce it where it doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@h-vetinari
Did not know that - Grayskull generates with name: {{ name|lower }}, and that's always prompted making the change - and I thought I was making things better / more standard by replacing with the variable. !

Copy link
Member

Choose a reason for hiding this comment

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

People can have different opinions which is better or worse, and though mine is pretty clear, it's certainly not the end of the world. The name variable is IMO an artefact of the automatisation approach (it's already a variable in grayskull, so the impulse to reuse it is there by default), but I haven't had the time to go fix it at the source. Probably I should one of these days...

Copy link
Member

Choose a reason for hiding this comment

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

Had started to in PR: conda/grayskull#548

Probably needs some tweaks before it can be merged

@h-vetinari h-vetinari merged commit ea6397e into conda-forge:main Aug 21, 2024
3 checks passed
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.

4 participants