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

Update GitLab CI/CD script #11

Merged
merged 3 commits into from
Jun 11, 2024
Merged

Update GitLab CI/CD script #11

merged 3 commits into from
Jun 11, 2024

Conversation

Polo2
Copy link
Member

@Polo2 Polo2 commented Jun 11, 2024

Some suggestions noticed by testing the script on a test repository on GitLab:

  • Update node to last version (15 -> 18)
  • Mention how to run CI/CD script without installing bump-cli package in 'package.json'
  • Replace default private branch (master -> main)
  • Complete calls to CLI: add parameters slug, token, and favor single quotes (or it crashs, cf this job )
  • Use sh to call 'diff-comment-mr.sh' and thus override execution rights missing (or it crashes, cf this job )

@Polo2
Copy link
Member Author

Polo2 commented Jun 11, 2024

cc @paulRbr , I've open this PR following yesterday discussions.

WDYT of these suggestions?

May I ask your help about why circleCI failed (no change in this PR at first sight) 🙏

@Polo2 Polo2 requested a review from paulRbr June 11, 2024 07:40
@Polo2 Polo2 self-assigned this Jun 11, 2024
.gitlab-ci.yml Outdated Show resolved Hide resolved
.gitlab-ci.yml Outdated

diff_doc:
stage: test
script:
- ./.gitlab/diff-comment-mr.sh bump.openapi.v3.yml
- ./.gitlab/diff-comment-mr.sh 'bump.openapi.v3.yml' bump_token
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding a comment in the sh script, you can call sh here directly which removes the necessity for the execution flag on the script file:

Suggested change
- ./.gitlab/diff-comment-mr.sh 'bump.openapi.v3.yml' bump_token
- sh ./.gitlab/diff-comment-mr.sh "bump.openapi.v3.yml" "bump_documentation_slug" "bump_token"

Copy link
Member Author

@Polo2 Polo2 Jun 11, 2024

Choose a reason for hiding this comment

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

Thanks for suggestion and comment, I've adapted script consequently.

  • run script with sh and remove comment about chmode
  • adapt call to this script, with 3 parameters "bump.openapi.v3.yml" "bump_documentation_slug" "bump_token"

@Polo2 Polo2 force-pushed the update-gitlab-cicd-script branch from f563981 to 5dc617f Compare June 11, 2024 11:12
@Polo2 Polo2 requested a review from paulRbr June 11, 2024 11:13
.gitlab-ci.yml Outdated Show resolved Hide resolved
Current script assumes that node package bump-cli is installed
in the application where this GitLab CI/CD script is used,
hence the presence of a 'package.json' configuration file.

But this script can also be run with more simplicity:
package 'bump-cli' is found and installed by npm during CI/CD process.
Add parameters documentation_slug and bump_token.
These can be defined with GitLab CI/CD variables:
https://docs.gitlab.com/ee/ci/variables/index.html#protect-a-cicd-variable

And add double quotes around 'path/to/specification',
required to run script on GitLab.

Call 'diff-comment-mr' file script with `sh`, no need to update rights to be executable.
@Polo2 Polo2 force-pushed the update-gitlab-cicd-script branch from 5dc617f to c66af19 Compare June 11, 2024 11:25
Copy link
Member

@paulRbr paulRbr left a comment

Choose a reason for hiding this comment

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

Everything seems to be fine on https://gitlab.com/bump-sh/bump-ci-example/-/merge_requests/3

let's merge this!

@Polo2 Polo2 merged commit 502b137 into main Jun 11, 2024
2 checks passed
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.

2 participants