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

chore(cd): update cd workflow #78

Merged
merged 7 commits into from
Jul 5, 2023
Merged

chore(cd): update cd workflow #78

merged 7 commits into from
Jul 5, 2023

Conversation

joshua-goldstein
Copy link
Contributor

@joshua-goldstein joshua-goldstein commented Jul 5, 2023

  • Use appropriate token name with granular access
  • Specify whether release is latest, beta, or rc (without this it will always default to latest)

Copy link

@kevinmingtarja kevinmingtarja left a comment

Choose a reason for hiding this comment

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

LGTM! Just one clarification regarding the token secret name change.

.github/workflows/cd-dgraph-js-http.yml Show resolved Hide resolved
@joshua-goldstein
Copy link
Contributor Author

LGTM! Just one clarification regarding the token secret name change.

Thanks for the quick review. The problem actually was with the token (either it was out of date or expired). I regenerated the token and it was published successfully to npm. The error message was just misleading.

However I noticed a different problem. :) Without specifying the version, it defaults to latest, which we do not want in the case of an rc like this. I added a field in the workflow to make this optional.

@joshua-goldstein joshua-goldstein merged commit bfcfbbf into master Jul 5, 2023
7 checks passed
@joshua-goldstein joshua-goldstein deleted the joshua/cd-fixes branch July 5, 2023 22:53
@kevinmingtarja
Copy link

LGTM! Just one clarification regarding the token secret name change.

Thanks for the quick review. The problem actually was with the token (either it was out of date or expired). I regenerated the token and it was published successfully to npm. The error message was just misleading.

However I noticed a different problem. :) Without specifying the version, it defaults to latest, which we do not want in the case of an rc like this. I added a field in the workflow to make this optional.

Ah i see, sounds good Josh. And yeah it makes sense to have the option for tags. LGTM!

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

2 participants