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

build: add detect-secrets stage to build #223

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Conversation

padamstx
Copy link
Member

@padamstx padamstx commented Jul 26, 2024

This PR re-organizes the Travis build a bit in order to add a "detect-secrets" stage to the build.

@padamstx padamstx force-pushed the add-detect-secrets branch 6 times, most recently from bd28767 to 1fe0d48 Compare July 26, 2024 19:51
@padamstx padamstx requested review from pyrooka and dpopp07 July 26, 2024 19:52
@padamstx padamstx self-assigned this Jul 26, 2024
Copy link
Member

@dpopp07 dpopp07 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Had a question about the approach but I'll approve to avoid re-review (though @pyrooka might want to chime in on Monday 🙂)

- pip install --upgrade "git+https://github.com/ibm/detect-secrets.git@master#egg=detect-secrets"
script:
- detect-secrets scan --update .secrets.baseline
- detect-secrets -v audit --report --fail-on-unaudited --fail-on-live --fail-on-audited-real .secrets.baseline
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, why not just add these steps to the above "script" section, where the tests are run, etc. ?

This looks good and looks clean overall, I'm just curious why you took this approach.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mainly so that everything is run in parallel. Under the "Build-Test" stage, there are four different jobs that each run in parallel:
image

Copy link
Member Author

Choose a reason for hiding this comment

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

And to carry this a little bit further (farther? 😂), when we merge the PR into main, we'll see the "Semantic-Release" stage get run also:
image

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay, cool!

.travis.yml Outdated
install:
- npm install
script:
- echo npm run semantic-release
Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I just realized I need to fix this 😂

Copy link
Member Author

Choose a reason for hiding this comment

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

now fixed

@padamstx padamstx force-pushed the add-detect-secrets branch from 1fe0d48 to 2f5ea19 Compare July 26, 2024 22:45
Copy link
Member

@pyrooka pyrooka left a comment

Choose a reason for hiding this comment

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

Nothing I can complain about (😂), this looks good and clean indeed.

@padamstx padamstx force-pushed the add-detect-secrets branch 2 times, most recently from db77906 to fe7f685 Compare July 29, 2024 15:44

Verified

This commit was signed with the committer’s verified signature.
cmackenzie1 Cole Mackenzie
Signed-off-by: Phil Adams <phil_adams@us.ibm.com>
@padamstx padamstx force-pushed the add-detect-secrets branch from fe7f685 to e449cf8 Compare July 29, 2024 16:56
@padamstx padamstx merged commit 393f95d into main Jul 29, 2024
4 checks passed
@padamstx padamstx deleted the add-detect-secrets branch July 29, 2024 20:44
@ibm-devx-sdk
Copy link

🎉 This PR is included in version 5.17.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants