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 DCI linting to CI of PRs #1393

Merged
merged 5 commits into from
Jun 28, 2022
Merged

Conversation

aklenik
Copy link
Contributor

@aklenik aklenik commented Jun 28, 2022

As recommended by the project update template and https://github.com/petermetz/gh-action-dci-lint
Let's see whether the remnant phrases in the changelog and third-party artifacts will trigger the alarm before making this a branch protection rule.

Signed-off-by: Attila Klenik [email protected]

Signed-off-by: Attila Klenik <[email protected]>
@aklenik aklenik requested a review from davidkel June 28, 2022 11:25
@@ -216,7 +216,8 @@ describe('When using a PrometheusPushTxObserver', () => {

await prometheusPushTxObserver.deactivate();

// Values should be zero, or empty (https://github.com/siimon/prom-client/blob/master/test/counterTest.js)
// Values should be zero, or empty
// Ref: https://github.com/siimon/prom-client/ file test/counterTest.js
Copy link
Contributor

Choose a reason for hiding this comment

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

this url doesn't look valid. If we can't use the real url then we should delete the line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's split into repo and file parts. I could structure it better if needed. But the entire file URL contains master...
Or we can remove it entirely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davidkel, could be structured like this:
See the "test/counterTest.js" file in https://github.com/siimon/prom-client/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -181,7 +181,8 @@ describe('When using a PrometheusTxObserver', () => {

await prometheusTxObserver.deactivate();

// Values should be zero, or empty (https://github.com/siimon/prom-client/blob/master/test/counterTest.js)
// Values should be zero, or empty
// Ref: https://github.com/siimon/prom-client/ file test/counterTest.js
Copy link
Contributor

Choose a reason for hiding this comment

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

this url doesn't look valid. If we can't use the real url then we should delete the line

Copy link
Contributor

@davidkel davidkel left a comment

Choose a reason for hiding this comment

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

just a couple of minor comments

Signed-off-by: Attila Klenik <[email protected]>
Copy link
Contributor

@davidkel davidkel left a comment

Choose a reason for hiding this comment

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

LGTM

@aklenik
Copy link
Contributor Author

aklenik commented Jun 28, 2022

@davidkel The DCO check is stuck, can we re-trigger it somehow?

Signed-off-by: Attila Klenik <[email protected]>
@aklenik aklenik merged commit 04a1154 into hyperledger-caliper:main Jun 28, 2022
@aklenik aklenik deleted the add-dci-to-ci branch June 28, 2022 15:12
eravatee pushed a commit to eravatee/caliper that referenced this pull request Oct 4, 2022
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