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

.github: Create lint-rst.yaml #16387

Merged
merged 1 commit into from Jul 2, 2021
Merged

.github: Create lint-rst.yaml #16387

merged 1 commit into from Jul 2, 2021

Conversation

geyslan
Copy link
Contributor

@geyslan geyslan commented Jun 1, 2021

This is a Github workflow for automatic checking and catching malformed
RST files on pushes.

It can also be manually triggered from the Github Actions UI for testing
with the following options:

  • rstcheck report type
  • RST files to find
  • Paths to find

Fixes: #13366

Signed-off-by: Geyslan G. Bem geyslan@accuknox.com

@geyslan geyslan requested review from a team as code owners June 1, 2021 20:45
@geyslan geyslan requested review from nebril and aanm June 1, 2021 20:45
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 1, 2021
@aanm aanm added the release-note/misc This PR makes changes that have no direct user impact. label Jun 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 2, 2021
Copy link
Member

@nebril nebril left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

As I understand GH Actions triggers, workflow_dispatch is a manually triggered event (https://docs.github.com/en/actions/reference/events-that-trigger-workflows#workflow_dispatch) and we would like to run this on all PRs (possibly marking it as required for merging a PR).

Please take a look at https://github.com/cilium/cilium/blob/master/.github/workflows/lint-bpf-checks.yaml as an example.

Edit: sorry, I misread your initial PR comment. I still think we should run this on PRs rather than on stable branch pushes.

.github/workflows/lint-rst.yaml Show resolved Hide resolved
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.9 Jun 4, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.11 Jun 4, 2021
@geyslan geyslan requested a review from a team as a code owner June 4, 2021 22:10
@geyslan geyslan requested a review from nebril June 21, 2021 12:37
Copy link
Member

@nebril nebril left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! One more change request and I think we're good to go here.

.github/workflows/lint-rst.yaml Outdated Show resolved Hide resolved
This is a Github workflow for automatic checking and catching malformed
RST files on PRs.

It can also be manually triggered from the Github Actions UI for testing
with the following options:

- rstcheck report type
- RST files to find
- Paths to find

Fixes: cilium#13366

Signed-off-by: Geyslan G. Bem <geyslan@accuknox.com>
@geyslan
Copy link
Contributor Author

geyslan commented Jun 25, 2021

Thanks for the changes! One more change request and I think we're good to go here.

Done! Force pushed. Thanks @nebril

@geyslan geyslan requested a review from nebril June 27, 2021 14:24
@aanm aanm merged commit 6eea0ad into cilium:master Jul 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.9 Jul 5, 2021
@geyslan
Copy link
Contributor Author

geyslan commented Jul 5, 2021

@twpayne I think the issue https://github.com/cilium/cilium/actions/runs/1000552313 is related to pull requests of new branches from witch we can't have SHA in ${{ github.event.before }} etc.

image

A solution would be to use ${{ github.event.pull_request.base.ref }} and ${{ github.event.pull_request.base.sha }} as mentioned here.

@twpayne
Copy link
Contributor

twpayne commented Jul 5, 2021

Thanks for investigating! So, if I understand correctly, line number 49:

git diff --name-only --diff-filter=ACMRT ${{ github.event.before }} ${{ github.event.after }} | grep '\.rst$' >> $GITHUB_ENV

should be replaced with

git diff --name-only --diff-filter=ACMRT ${{ github.event.pull_request.base.ref }} ${{ github.event.pull_request.base.sha }} | grep '\.rst$' >> $GITHUB_ENV

Similarly, line 24, currently:

  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.event.after }}

should be replaced with:

  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.event.pull_request.base.sha }}

Is this correct?

@qmonnet
Copy link
Member

qmonnet commented Jul 5, 2021

I'm not sure I understand, what does this PR brings that b381918 did not?

@geyslan
Copy link
Contributor Author

geyslan commented Jul 5, 2021

I did some tests by sending PRs to my forked cilium.

https://github.com/geyslan/cilium/runs/2991422456?check_suite_focus=true

It seemed to work changing, as you proposed, line number 24 to:

group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.event.pull_request.base.sha }}

and line 49 to:

git diff --name-only --diff-filter=ACMRT ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }} | grep '\.rst$' >> $GITHUB_ENV

The documentation states that base.ref is the destiny branch name (master).

@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Needs backport from master in 1.8.11 Jul 6, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Needs backport from master in 1.9.9 Jul 6, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Needs backport from master in 1.9.9 Jul 6, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Needs backport from master in 1.9.9 Jul 6, 2021
joestringer pushed a commit that referenced this pull request Jul 7, 2021
This reverts commit 6eea0ad, which
seems to have broken the build, until it is investigated further.

References #16387.

Signed-off-by: Bruno Miguel Custódio <brunomcustodio@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Catch malformed releases table before merging
5 participants