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

Adding publish step in ci #13

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

ankush-cohere
Copy link
Contributor

@ankush-cohere ankush-cohere commented Jul 18, 2024

I have added a step to publish compass-sdk on pypi. This is a step similar to how we release cohere-python package.

@ankush-cohere ankush-cohere requested a review from a team as a code owner July 18, 2024 09:56
Copy link

cla-assistant bot commented Jul 18, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

cla-assistant bot commented Jul 18, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ankush-cohere ankush-cohere force-pushed the adding_publish_step_ci branch 2 times, most recently from 357d2d4 to ff3856f Compare July 18, 2024 10:25
Copy link
Contributor

@benrules3 benrules3 left a comment

Choose a reason for hiding this comment

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

This change looks good to me, great to publish to PyPi - can you explain how the package versioning gets defined though? Not clear on that part, and want to make sure it's sensible.

@ankush-cohere
Copy link
Contributor Author

ankush-cohere commented Jul 18, 2024

If I am not wrong, it should be the same version as specified in pyproject.toml

@benrules3
Copy link
Contributor

If I am not wrong, it should be the same version as specified in pyproject.toml

If that is the case, I think we should change from 0.1.0 to 1.0.0 or 0.2.0 at least. We should define the dev workflow when making changes too, for example if I add something new do I need to manual increment a minor version?

@ankush-cohere ankush-cohere force-pushed the adding_publish_step_ci branch from ff3856f to 3d6a5c7 Compare July 18, 2024 13:35
@ankush-cohere
Copy link
Contributor Author

If that is the case, I think we should change from 0.1.0 to 1.0.0 or 0.2.0 at least.

let's start with 1.0.0

We should define the dev workflow when making changes too, for example if I add something new do I need to manual
increment a minor version?

It should be the standard, 1.0.+ for bug fix, 1.+.0 for new features and +.0.0 for breaking changes

benrules3
benrules3 previously approved these changes Jul 18, 2024
Copy link
Contributor

@benrules3 benrules3 left a comment

Choose a reason for hiding this comment

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

It should be the standard, 1.0.+ for bug fix, 1.+.0 for new features and +.0.0 for breaking changes

This feels like it should be a github action or pre-commit hook. Basically check every PR includes a version change, and include the criteria for each type. Can come as another PR, but do you agree? And how would you enforce this?

@ankush-cohere
Copy link
Contributor Author

Now if I think about it, isn't that what a tag is for? Maybe picking the version via tag is a better option. Ideally consistency between pyproject.toml and tag would be fantastic.

@ankush-cohere ankush-cohere force-pushed the adding_publish_step_ci branch from bc1e311 to 541610e Compare October 10, 2024 12:44
@ankush-cohere ankush-cohere force-pushed the adding_publish_step_ci branch from 40a5a69 to 7e3d586 Compare October 10, 2024 16:06
@ankush-cohere ankush-cohere force-pushed the adding_publish_step_ci branch from 7e3d586 to bb00428 Compare October 10, 2024 16:09
Copy link
Contributor

@benrules3 benrules3 left a comment

Choose a reason for hiding this comment

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

LGTM - I defer to @miguelvr for final approve here though

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.

3 participants