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

Run module publish action only on the main repo #3153

Merged
merged 1 commit into from
Apr 15, 2020

Conversation

onursumer
Copy link
Member

@onursumer onursumer commented Apr 15, 2020

Fix cBioPortal/cbioportal/issues/6970

Checks

  • Has tests or has a separate issue that describes the types of test that should be created. If no test is included it should explicitly be mentioned in the PR why there is no test.
  • The commit log is comprehensible. It follows 7 rules of great commit messages. For most PRs a single commit should suffice, in some cases multiple topical commits can be useful. During review it is ok to see tiny commits (e.g. Fix reviewer comments), but right before the code gets merged to master or rc branch, any such commits should be squashed since they are useless to the other developers. Definitely avoid merge commits, use rebase instead.
  • Is this PR adding logic based on one or more clinical attributes? If yes, please make sure validation for this attribute is also present in the data validation / data loading layers (in backend repo) and documented in File-Formats Clinical data section!

@onursumer onursumer added the cl-bug Bug section of changelog. Bug fix label Apr 15, 2020
@onursumer onursumer requested review from inodb and zhx828 April 15, 2020 03:46
@@ -10,6 +10,12 @@
# Prompt changed packages to manually set the new version

if [[ "$GITHUB_RUN_ID" ]]; then
# do not attempt versioning if the NODE_AUTH_TOKEN is missing
if [[ -z "$NODE_AUTH_TOKEN" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Just a note: this is under assumption that no one has NODE_AUTH_TOKEN except the people actually wants to publish it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like there is some sort of placeholder for NODE_AUTH_TOKEN under forked repos as well. So, this check does NOT work. We need to come up with a better solution.

Copy link
Member Author

@onursumer onursumer Apr 15, 2020

Choose a reason for hiding this comment

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

Now using a job level conditional within the workflow yaml file. It seems tricky to check secret variables, so for now just checking the repository name.

@onursumer onursumer marked this pull request as draft April 15, 2020 04:15
Signed-off-by: Onur Sumer <s.onur.sumer@gmail.com>
@onursumer onursumer changed the title Check NPM token before auto versioning Run module publish action only on the main repo Apr 15, 2020
@onursumer onursumer marked this pull request as ready for review April 15, 2020 05:27
@onursumer onursumer requested a review from zhx828 April 15, 2020 06:37
Copy link
Member

@inodb inodb left a comment

Choose a reason for hiding this comment

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

Nice find 👍

@onursumer onursumer merged commit 47eaa4e into cBioPortal:master Apr 15, 2020
@onursumer onursumer deleted the npm-token-check-gha branch April 15, 2020 13:32
inodb pushed a commit to inodb/cbioportal-frontend that referenced this pull request Jan 12, 2022
Run module publish action only on the main repo

Former-commit-id: 47eaa4e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cl-bug Bug section of changelog. Bug fix
Projects
None yet
3 participants