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

Remove dist folder and binary file. Ignore dist in gitignore #2

Merged
merged 1 commit into from
Jan 23, 2020

Conversation

kinow
Copy link
Contributor

@kinow kinow commented Oct 25, 2019

Hi,

Not sure if the dist/ folder is being intentionally kept with a binary (2.8MB) with the sources?

It is probably in PYPI as well, so I think it doesn't have to be part of the git repository? Or, if intended, this could be uploaded to the GitHub release instead too?

This PR removes the binary, and also marks it as ignored in .gitignore.

Thanks for cf-python 👍
Bruno

@davidhassell
Copy link
Collaborator

Hi Bruno,

The good news is the the dist/ directory is not in the PyPi tar ball.

I do want to keep cf-python-1.5.4.post6.tar.gz as it was the last release before a major API change, and was developed in different repository that may not exist for ever.

I looked at attaching it to a GitHub release, but it appears that I can only attach the binary to a snapshot of the current repo, which is not really how I want to store it.

I'm going to think about it for a bit ... more suggestions welcome!

All the best,
David

@kinow
Copy link
Contributor Author

kinow commented Oct 25, 2019

Hi David,

Thanks for the quick reply.

I looked at attaching it to a GitHub release, but it appears that I can only attach the binary to a snapshot of the current repo, which is not really how I want to store it.

Ah, I see. Perhaps it would be possible to upload this version to PYPI with Twine (maybe test using test.pypi.org, by simply running twine upload --repository-url https://test.pypi.org/legacy/ dist/* as in their example.

Or create a tag in GitHub for that old release and upload the binary there. This way users could compare the code either after downloading the binary from the tag, or using something like git diff v3.0.1 v1.5.4.

Not really important I guess as doesn't stop users of using the code which is the most important :o)

@sadielbartholomew
Copy link
Member

Hi @kinow (nice to converse with you on GH again!),

Sorry for the delay. Replying on behalf of @davidhassell who's on leave, having had time to reconsider he thinks it is sufficient to have the binary up on PyPI, & I agree, so we are happy to merge. You've also initiated a solution towards #21, which is great. Thanks for both contributions.

@sadielbartholomew sadielbartholomew merged commit 8c4a9a9 into NCAS-CMS:master Jan 23, 2020
@kinow
Copy link
Contributor Author

kinow commented Jan 23, 2020

Always great to work with you @sadielbartholomew ! Thanks!

davidhassell added a commit that referenced this pull request May 23, 2023
Move concatenation out of `_aggregate_2_fields`
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

3 participants