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

Add support for publishing Debian packages #1680

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

WadeBarnes
Copy link
Member

Signed-off-by: Wade Barnes [email protected]

@WadeBarnes
Copy link
Member Author

(ci) test this please

6 similar comments
@WadeBarnes
Copy link
Member Author

(ci) test this please

@WadeBarnes
Copy link
Member Author

(ci) test this please

@WadeBarnes
Copy link
Member Author

(ci) test this please

@WadeBarnes
Copy link
Member Author

(ci) test this please

@WadeBarnes
Copy link
Member Author

(ci) test this please

@WadeBarnes
Copy link
Member Author

(ci) test this please

@WadeBarnes WadeBarnes force-pushed the publishing-artifacts branch from dd8da4d to 3888811 Compare August 4, 2021 20:58
Copy link
Contributor

@udosson udosson left a comment

Choose a reason for hiding this comment

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

Thanks @WadeBarnes for this PR.
In general, it looks good to me. There are just adjustments left and maybe the base image of node-lint has to be changed.

.github/actions/set-version/action.yaml Outdated Show resolved Hide resolved
.github/workflows/README.md Outdated Show resolved Hide resolved
run: |
DOCKER_IMAGE=ghcr.io/${{ env.GITHUB_REPOSITORY_NAME }}/node-lint
# ToDo - Update hard coded 'ubuntu-18-04' tag when integrating these flows with the ubuntu-20.04-upgrade branch.
TAGS="${DOCKER_IMAGE}:latest,${DOCKER_IMAGE}:ubuntu-18-04"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why we base the node-lint image on ubuntu:18.04 instead of ubuntu:16.04?
I know that it's the same for indy-plenum. Just wondering why

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 OS version for the lint image is less important than the version of the tools installed for linting. It looks like it was a matter of choice when the dockerfile was originally created.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed

.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
@@ -1,6 +1,6 @@
# Development
FROM ubuntu:18.04
Copy link
Contributor

Choose a reason for hiding this comment

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

From ubuntu:16.04 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since it works with 18.04 I'm inclined to leave it. The OS version for the lint image is less important than the version of the tools installed for linting.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed

@WadeBarnes
Copy link
Member Author

(ci) test this please

@WadeBarnes WadeBarnes force-pushed the publishing-artifacts branch from 0cddc67 to f6f72f1 Compare August 5, 2021 17:17
@WadeBarnes
Copy link
Member Author

(ci) test this please

@WadeBarnes WadeBarnes force-pushed the publishing-artifacts branch 2 times, most recently from 1b82b9b to 36f3089 Compare August 6, 2021 18:44
- Following the pattern and enhancements developed for the plenum workflows.
- Update Sovrin signing keys.
- Update test runner (`runner.py`) to automatically report the test results for each set of tests.
  - Enhancements from hyperledger/indy-plenum@06d9406
  - Add an option for suppressing the automated junitxml results.
    - Add `--nojunitxml` flag to allow junitxml results to be suppressed.
    - Suppress junitxml output when running the Jenkins Pipelines
      - Generation of junitxml output for the `indy_node/test/auth_rule` tests causes the tests to crash and fail when run via Jenkins.
    - The GHA workflows use the junitxml results for reporting, and are not affected by the same issue seen with Jenkins.
- Address feedback from @udosson

Signed-off-by: Wade Barnes <[email protected]>
@WadeBarnes WadeBarnes force-pushed the publishing-artifacts branch from 36f3089 to 0730c69 Compare August 6, 2021 18:47
@WadeBarnes
Copy link
Member Author

(ci) test this please

@WadeBarnes WadeBarnes requested a review from udosson August 6, 2021 19:00
@WadeBarnes
Copy link
Member Author

@udosson, ready for another review. I've also resolved the issues with the Jenkins pipelines. It was the added junitxml reports, which we use for the GHA workflows that were causing the tests to blow up when running on Jenkins.

Copy link
Contributor

@udosson udosson 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 :)

@@ -1,6 +1,6 @@
# Development
FROM ubuntu:18.04
Copy link
Contributor

Choose a reason for hiding this comment

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

agreed

run: |
DOCKER_IMAGE=ghcr.io/${{ env.GITHUB_REPOSITORY_NAME }}/node-lint
# ToDo - Update hard coded 'ubuntu-18-04' tag when integrating these flows with the ubuntu-20.04-upgrade branch.
TAGS="${DOCKER_IMAGE}:latest,${DOCKER_IMAGE}:ubuntu-18-04"
Copy link
Contributor

Choose a reason for hiding this comment

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

agreed

@WadeBarnes WadeBarnes merged commit ccbd47d into hyperledger:master Aug 10, 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.

2 participants