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

Fixed updating the library version in the CD workflow #970

Merged
merged 2 commits into from
Feb 25, 2024

Conversation

barshaul
Copy link
Collaborator

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@barshaul barshaul requested a review from a team as a code owner February 15, 2024 12:57
@barshaul barshaul marked this pull request as draft February 15, 2024 12:57
@barshaul barshaul force-pushed the client_version branch 6 times, most recently from b07e427 to cdb8ca8 Compare February 15, 2024 14:30
@barshaul barshaul marked this pull request as ready for review February 15, 2024 14:53
Copy link
Contributor

@shachlanAmazon shachlanAmazon left a comment

Choose a reason for hiding this comment

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

when are the CD workflow called by a PR?

.github/workflows/npm-cd.yml Outdated Show resolved Hide resolved
@Yury-Fridlyand
Copy link
Collaborator

Please add a repo check as it was done in #948 for another workflow

@barshaul
Copy link
Collaborator Author

barshaul commented Feb 18, 2024

when are the CD workflow called by a PR?

When changes are done to
- .github/workflows/pypi-cd.yml (or npm-cd.yml)
- .github/workflows/build-python-wrapper/action.yml

to verify that it doesn't break the CD

@barshaul
Copy link
Collaborator Author

@shachlanAmazon round

.github/workflows/npm-cd.yml Outdated Show resolved Hide resolved
.github/workflows/npm-cd.yml Show resolved Hide resolved
@@ -68,25 +69,30 @@ jobs:
cat Cargo.toml

- name: Set up Python
if: matrix.build.PYPI_PUBLISH == true && !contains(matrix.build.RUNNER, 'self-hosted')
if: ${{ !contains(matrix.build.RUNNER, 'self-hosted') }}
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: why do you need the curly braces here and not in lines 78 / 83?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The linter gets angry when it's with ! (not) :(

.github/workflows/pypi-cd.yml Outdated Show resolved Hide resolved
.github/workflows/pypi-cd.yml Outdated Show resolved Hide resolved
@barshaul barshaul force-pushed the client_version branch 2 times, most recently from 05896e6 to 2b4cb04 Compare February 25, 2024 11:17
@barshaul barshaul merged commit d651379 into valkey-io:main Feb 25, 2024
41 checks passed
@barshaul barshaul deleted the client_version branch February 25, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI CI/CD related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants