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

Allow applying fixes on push events #487

Merged
merged 8 commits into from
May 25, 2021
Merged

Conversation

vkucera
Copy link
Contributor

@vkucera vkucera commented May 25, 2021

Partially fixes issue #481

Proposed Changes

  1. Modify the condition for executing the following workflow steps:
  • Create Pull Request with applied fixes
  • Create PR output
  • Prepare commit
  • Commit and push applied linter fixes

so that it can be satisfied for push events.

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@vkucera vkucera requested a review from nvuillam as a code owner May 25, 2021 00:00
Copy link
Member

@nvuillam nvuillam left a comment

Choose a reason for hiding this comment

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

Great, thanks a lot for your contribution :)

@nvuillam nvuillam enabled auto-merge (squash) May 25, 2021 05:50
@nvuillam nvuillam disabled auto-merge May 25, 2021 05:51
@nvuillam
Copy link
Member

@vkucera there is an update to add to cspell.json before I can merge :) (you can just take generated cspell.json file in Mega-Linter artifacts then commit & push )
If you are in a good mood, you can also complete the doc with the missing points you noticed :) ( else i'll do that later)

@codecov-commenter
Copy link

Codecov Report

Merging #487 (8fe0c08) into master (c0824e6) will increase coverage by 0.27%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #487      +/-   ##
==========================================
+ Coverage   87.07%   87.34%   +0.27%     
==========================================
  Files         127      127              
  Lines        2956     2956              
==========================================
+ Hits         2574     2582       +8     
+ Misses        382      374       -8     
Impacted Files Coverage Δ
megalinter/MegaLinter.py 90.65% <0.00%> (+2.42%) ⬆️
megalinter/reporters/UpdatedSourcesReporter.py 89.18% <0.00%> (+2.70%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0824e6...8fe0c08. Read the comment docs.

@vkucera
Copy link
Contributor Author

vkucera commented May 25, 2021

Hi @nvuillam , I modified also other occurrences of the same condition in files that I had overlooked initially. Can you please check that the changes are appropriate?
I added the updated .cspell.json but the error did not disappear. I guess it might be due to the diacritics in my name so I replaced the Unicode code of the character with the character itself. Let's see if the test passes now.
I also fixed the missing file extension in the contribution instructions.
Regarding the node.js version, I don't know which is the minimum required one so I couldn't add any meaningful instructions there. But if you tell me the version, I can add it in this PR.

@nvuillam
Copy link
Member

I'm quite sure node14 will be ok :)

@vkucera
Copy link
Contributor Author

vkucera commented May 25, 2021

I'm quite sure node14 will be ok :)

Done! :-)

@nvuillam nvuillam merged commit 5f5959d into oxsecurity:master May 25, 2021
@nvuillam
Copy link
Member

@vkucera all good, thanks a lot for your contribution and your updates, and welcome to the team :)

@vkucera vkucera deleted the apply-fixes branch May 25, 2021 21:25
@vkucera
Copy link
Contributor Author

vkucera commented May 25, 2021

Thanks!

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

3 participants