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

[SCB-2205] Polishing Github actions workflow #703

Merged
merged 3 commits into from Mar 16, 2021

Conversation

coolbeevip
Copy link
Member

@coolbeevip coolbeevip commented Mar 3, 2021

  • Splits the ci.yaml to pr and push script
  • Temporarily disable unstable test case for poor performance CI
  • Update GHA script for JIRA's recommendations in the GitHub Action Status
  • Add Coveralls
  • Add scheduled deploy workflow
  • Remove Travis

@coolbeevip coolbeevip changed the title WIP: [SCB-2205] Fix pull request and push trigger condition WIP: [SCB-2205] Polishing Github operation workflow Mar 3, 2021
@coolbeevip coolbeevip changed the title WIP: [SCB-2205] Polishing Github operation workflow WIP: [SCB-2205] Polishing Github actions workflow Mar 3, 2021
@coveralls
Copy link

coveralls commented Mar 3, 2021

Coverage Status

Coverage decreased (-0.02%) to 80.677% when pulling 1645d2d on coolbeevip:SCB-2205 into 6033905 on apache:master.

@coolbeevip coolbeevip closed this Mar 4, 2021
@coolbeevip coolbeevip reopened this Mar 4, 2021
@coolbeevip coolbeevip force-pushed the SCB-2205 branch 2 times, most recently from e0204a5 to fbceed2 Compare March 16, 2021 01:18
@coolbeevip coolbeevip changed the title WIP: [SCB-2205] Polishing Github actions workflow [SCB-2205] Polishing Github actions workflow Mar 16, 2021
@chanjarster
Copy link
Member

LGTM

run: git log -n1
- name: Build and test
if: ${{ success() }}
run: ./mvnw clean install -B -Pjacoco -Pdocker coveralls:report -DrepoToken=ftogh57jTQ0GRa4AaOgFfltiUrwnO8tw9
Copy link
Contributor

Choose a reason for hiding this comment

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

The token string should be used in environment variables ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The token string should be used in environment variables ?

secrets are not passed to the runner when a workflow is triggered from a forked repository.

As GitHub stuff answered here

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I see. so is there any other problem we provide the token in plain text here ? this token could be changed in the future ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The token has been set to GitHub secrets with https://issues.apache.org/jira/browse/INFRA-21569 and uses secrets.COVERALLS_TOKEN in push workflow.

If we do not analyze the coverage in the pull request, we can remove the plaintext token

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, I prefer to disable the coverage in PR now and see if we can have a better solution in the future. How do you think @WillemJiang

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm, I prefer to disable the coverage in PR now and see if we can have a better solution in the future. How do you think @WillemJiang

I removed the Coveralls report token from the pull request script

jobs:
mvn:
timeout-minutes: 180
runs-on: ubuntu-20.04
Copy link
Contributor

Choose a reason for hiding this comment

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

why we stick on this ubuntu version 20.04, it can be the 'lastest' ?

Copy link
Member

Choose a reason for hiding this comment

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

it's better to specify the ubuntu version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, Ubuntu-latest workflows will use Ubuntu-20.04

# | 1 | Sean | 5 | true | false |
#
# Then Hotel Service contains the following booking orders
# | id | name | amount | confirmed | cancelled |
Copy link
Contributor

Choose a reason for hiding this comment

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

why we disable this scenario ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is an unstable test case, I created a JIRA https://issues.apache.org/jira/browse/SCB-2204 for it

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks - please add the JIRA issue link here and we can track it easier

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks - please add the JIRA issue link here and we can track it easier

done

Occasionally fail on poor performance CI.
I created a JIRA https://issues.apache.org/jira/browse/SCB-2204 for it
Copy link
Contributor

@zhfeng zhfeng left a comment

Choose a reason for hiding this comment

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

Great job !

@zhfeng zhfeng merged commit 99304ad into apache:master Mar 16, 2021
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

5 participants