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

Testing: git push auto fixed files #4160

Conversation

lukelloyd1985
Copy link
Contributor

Proposed Changes

Testing git add, commit & push when there are fixed sources files

Readiness Checklist

Author/Contributor

  • Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • 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

@@ -63,6 +64,11 @@ def produce_report(self):
f" in folder {updated_sources_dir}.\n"
"Download it from artifacts then copy-paste it in your local repo to apply linters updates"
)

repo = git.Repo(os.path.realpath(self.github_workspace))
Copy link
Member

Choose a reason for hiding this comment

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

Do that only if APPLY_FIXES = all , or matches the linter :)

  • self.master will give you the Linter instance
  • self.master.master will give you the MegaLinter instance
  • so self.master.master.github_workspace will work :)

Also, check if you are on GitHub: if you are, don't make git operation, as they are already defined in the next GitHub Action :) ( if it works, we can deprecate it in a next major version)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how to check the git platform being used?

Copy link
Member

Choose a reason for hiding this comment

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

if GITHUB_REPOSITORY is set, you're on GitHub actions :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

think I've catered for both of those now? 🤞

repo = git.Repo(os.path.realpath(self.github_workspace))
repo.git.add(update=True)
repo.index.commit("megalinter auto fixes")
repo.git.push
Copy link
Member

Choose a reason for hiding this comment

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

Also add try except, I don't want the whole MegaLinter to crash in case UpdatedSourcesReporter crashes, I prefer to just have a warning message ^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

put a try except around which will hopefully give the correct git error message as I can't imagine this is going to work first attempt 😆

apply_fixes = config.get_list(self.request_id, "APPLY_FIXES", "none")
if apply_fixes:
try:
repo = git.Repo(os.path.realpath(self.master.master.github_workspace))
Copy link
Member

Choose a reason for hiding this comment

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

sorry, master is MegaLinter instance, so it will be self.master.github_workspace :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

switched 😄

@@ -63,6 +64,16 @@ def produce_report(self):
f" in folder {updated_sources_dir}.\n"
"Download it from artifacts then copy-paste it in your local repo to apply linters updates"
)

apply_fixes = config.get_list(self.request_id, "APPLY_FIXES", "none")
Copy link
Member

Choose a reason for hiding this comment

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

self.master.request_id :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I overlooked that 🤦‍♂️ good catch 👍

@lukelloyd1985
Copy link
Contributor Author

@nvuillam, how come the PR got closed rather than merged? Did I do something wrong?

@nvuillam
Copy link
Member

@lukelloyd1985 I played with alpha to solve CI issues (which are still not solved, damn docker !)

So I deleted and recreated (then merged alpha), I think your PR has been closed when I deleted alpha branch to recreate it , sorry you can reopen it ! :)

@lukelloyd1985
Copy link
Contributor Author

@lukelloyd1985 I played with alpha to solve CI issues (which are still not solved, damn docker !)

So I deleted and recreated (then merged alpha), I think your PR has been closed when I deleted alpha branch to recreate it , sorry you can reopen it ! :)

Need a new alpha branch by the looks of it :)

@nvuillam
Copy link
Member

@lukelloyd1985 created :)
(can't promise it won't move, the CI issues are still not solved :'( :'( )

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