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.

Sign up for GitHub

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

Jump to bottom

ci(actionlint): exclude files with very large number of linter errors #3446

Merged
merged 1 commit into from
Aug 7, 2024
Merged

ci(actionlint): exclude files with very large number of linter errors #3446

merged 1 commit into from
Aug 7, 2024

Conversation

petermetz
Copy link
Member

  1. There is a set of yaml workflow files that have to be removed prior to the
    action lint task because they have hundreds of unfixed errors that we didn't
    yet have time to address and the action lint github action has no easy way
    to ignore files unfortunately so the easiest way to achieve that is to wipe
    the fiels we don't want linting. This workaround caused a problem with the
    workflow reuse improvements we were recently trying to introduce. So now
    the way they get excluded is that we feed an explicit list of workflow files
    to the linter that we want to get linted and we exclued the problematic files
    from that list.
  2. Also fixed on linter warning with the docker login action being outdated.

Signed-off-by: Peter Somogyvari [email protected]

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

@petermetz petermetz enabled auto-merge (rebase) July 26, 2024 19:29
@petermetz petermetz requested a review from RafaelAPB July 26, 2024 19:30
petermetz pushed a commit to jenniferlianne/cacti that referenced this pull request Jul 26, 2024
@petermetz
refactor(ci/github): run weaver tests on schedule, re-use workflows
a1834cb 
Primary change:

Creates a central workflow to
run all 13 weaver test workflows from either a
push, a pull request, a github command,
or a schedule.

Passes a 'run_all' boolean to sub-workflows
to specify if all tests should be run, not
just those with changes.

Secondary changes:
- update concurrency groups to allow triggering
  from central workflow
- remove ability to run the 13 workflows
  individually to declutter ci menu

check whether RUN_ALL can be calculated in the environment instead of as a job

rename weaver test file

update actionlint to latest

Depends on hyperledger#3446

Signed-off-by: Jennifer Bell <[email protected]>
@petermetz petermetz mentioned this pull request Jul 26, 2024
Merged
3 tasks
outSH
outSH requested changes Jul 29, 2024
View reviewed changes
.github/workflows/actionlint.yaml Outdated Show resolved Hide resolved
RafaelAPB
RafaelAPB approved these changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@RafaelAPB RafaelAPB left a comment

Choose a reason for hiding this comment

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

Besides Michal's comments, everything LGTM

@petermetz petermetz force-pushed the ci-actionlint-refactor-file-exclusion-logic-use-find-bash branch from daeb831 to 09ef9f2 Compare July 30, 2024 14:37
@petermetz petermetz requested a review from outSH July 30, 2024 14:38
outSH
outSH approved these changes Aug 6, 2024
View reviewed changes
Copy link
Contributor

@outSH outSH left a comment

Choose a reason for hiding this comment

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

@petermetz @jenniferlianne Sorry for keeping this blocked, you could've merge this once other contributors approved this, I was unavailable for last few days :(

@petermetz @sandeepnRES
ci(actionlint): exclude files with very large number of linter errors
fddc2c3 
1. There is a set of yaml workflow files that have to be removed prior to the
action lint task because they have hundreds of unfixed errors that we didn't
yet have time to address and the action lint github action has no easy way
to ignore files unfortunately so the easiest way to achieve that is to wipe
the fiels we don't want linting. This workaround caused a problem with the
workflow reuse improvements we were recently trying to introduce. So now
the way they get excluded is that we feed an explicit list of workflow files
to the linter that we want to get linted and we exclued the problematic files
from that list.
2. Also fixed on linter warning with the docker login action being outdated.

Signed-off-by: Peter Somogyvari <[email protected]>
@sandeepnRES sandeepnRES force-pushed the ci-actionlint-refactor-file-exclusion-logic-use-find-bash branch from 09ef9f2 to fddc2c3 Compare August 7, 2024 06:45
@petermetz petermetz merged commit aadaca3 into hyperledger:main Aug 7, 2024
140 of 143 checks passed
sandeepnRES pushed a commit to jenniferlianne/cacti that referenced this pull request Aug 7, 2024
@sandeepnRES
refactor(ci/github): run weaver tests on schedule, re-use workflows
e8763fd 
Primary change:

Creates a central workflow to
run all 13 weaver test workflows from either a
push, a pull request, a github command,
or a schedule.

Passes a 'run_all' boolean to sub-workflows
to specify if all tests should be run, not
just those with changes.

Secondary changes:
- update concurrency groups to allow triggering
  from central workflow
- remove ability to run the 13 workflows
  individually to declutter ci menu

check whether RUN_ALL can be calculated in the environment instead of as a job

rename weaver test file

update actionlint to latest

Depends on hyperledger#3446

Signed-off-by: Jennifer Bell <[email protected]>
sandeepnRES pushed a commit that referenced this pull request Aug 7, 2024
@sandeepnRES
refactor(ci/github): run weaver tests on schedule, re-use workflows
aa90ec6 
Primary change:

Creates a central workflow to
run all 13 weaver test workflows from either a
push, a pull request, a github command,
or a schedule.

Passes a 'run_all' boolean to sub-workflows
to specify if all tests should be run, not
just those with changes.

Secondary changes:
- update concurrency groups to allow triggering
  from central workflow
- remove ability to run the 13 workflows
  individually to declutter ci menu

check whether RUN_ALL can be calculated in the environment instead of as a job

rename weaver test file

update actionlint to latest

Depends on #3446

Signed-off-by: Jennifer Bell <[email protected]>
@petermetz petermetz deleted the ci-actionlint-refactor-file-exclusion-logic-use-find-bash branch August 7, 2024 21:24
@petermetz
Copy link
Member Author

@petermetz @jenniferlianne Sorry for keeping this blocked, you could've merge this once other contributors approved this, I was unavailable for last few days :(

@outSH No worries and thank you for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenniferlianne jenniferlianne jenniferlianne left review comments

@outSH outSH outSH approved these changes

@RafaelAPB RafaelAPB RafaelAPB approved these changes

@sandeepnRES sandeepnRES sandeepnRES approved these changes

@jagpreetsinghsasan jagpreetsinghsasan jagpreetsinghsasan approved these changes

@takeutak takeutak Awaiting requested review from takeutak takeutak is a code owner

@izuru0 izuru0 Awaiting requested review from izuru0 izuru0 is a code owner

@VRamakrishna VRamakrishna Awaiting requested review from VRamakrishna VRamakrishna is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@petermetz @jenniferlianne @outSH @RafaelAPB @sandeepnRES @jagpreetsinghsasan