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 verify check to handle new pom files [skip ci] #9657

Merged

Conversation

NVnavkumar
Copy link
Collaborator

Fixes #9650.

This updates the github workflow check to properly check for newer build files that needed to be added for Scala 2.13 and shows the diff properly for those files and any changed files.

@NVnavkumar
Copy link
Collaborator Author

build

Signed-off-by: Navin Kumar <navink@nvidia.com>
@NVnavkumar
Copy link
Collaborator Author

build

Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

Minor simplification suggestions otherwise lgtm.

Comment on lines +167 to +168
if [ -n "$(echo -n $(git status -s | grep 'scala2.13'))" ]; then
git add -N scala2.13/* && git diff 'scala2.13/*'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [ -n "$(echo -n $(git status -s | grep 'scala2.13'))" ]; then
git add -N scala2.13/* && git diff 'scala2.13/*'
if [[ -n $(git status --porcelain scala2.13) ]]; then
git add -N scala2.13 && git diff scala2.13

@gerashegalov gerashegalov changed the title Update verify check to handle new pom files Update verify check to handle new pom files [skip ci] Nov 8, 2023
@@ -164,7 +164,8 @@ jobs:
# verify Scala 2.13 build files
./build/make-scala-version-build-files.sh 2.13
# verify git status
if ! git diff --exit-code 'scala2.13/*'; then
if [ -n "$(echo -n $(git status -s | grep 'scala2.13'))" ]; then
git add -N scala2.13/* && git diff 'scala2.13/*'
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: * is unnecessary for git add

As a more compact fix we could just insert before L167 in the current code

git add -N scala2.13

which will be a NOP if there are no untracked files without changing anything else

Changes to this file don't affect the internal CI so adding [skip ci] to the PR title to save resources.

Copy link
Collaborator

@NvTimLiu NvTimLiu left a comment

Choose a reason for hiding this comment

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

LGTM

@NVnavkumar NVnavkumar merged commit 8040706 into NVIDIA:branch-23.12 Nov 9, 2023
37 checks passed
@sameerz sameerz added bug Something isn't working build Related to CI / CD or cleanly building labels Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build Related to CI / CD or cleanly building
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Github workflow for missing scala2.13 updates fails to detect when pom is new
5 participants