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

fix: ANR watcher wont crash on sec excep #1962

Merged
merged 3 commits into from
Mar 29, 2022
Merged

Conversation

bruno-garcia
Copy link
Member

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

@marandaneto
Copy link
Contributor

@bruno-garcia run make format

@bruno-garcia
Copy link
Member Author

@bruno-garcia run make format

Can we not break the build due to linting? We can have an action that runs this and pushes to the branch like we do in Cocoa, .NET and Unity?

Breaking CI due to linting is a huge productivity killer

@bruno-garcia
Copy link
Member Author

#1963

@marandaneto marandaneto merged commit 5036a47 into main Mar 29, 2022
@marandaneto marandaneto deleted the fix/anr-watchdog-crash branch March 29, 2022 06:24
@marandaneto
Copy link
Contributor

@bruno-garcia run make format

Can we not break the build due to linting? We can have an action that runs this and pushes to the branch like we do in Cocoa, .NET and Unity?

Breaking CI due to linting is a huge productivity killer

Good idea, but it was never a problem if you have the git pre-commit hooks installed or triggered build locally before as well, all of them were invoking the format automatically.
Thanks for the PR, I did some improvements and merged it into your PR as well, #1964 (comment)

@bruno-garcia
Copy link
Member Author

Fair point. pre-commit hooks are optional and new contributors might not have it setup or know how it works so the final PR bot can help in those cases. By reducing the barrier of entry for new contributors.

Thanks for merging this 🙏

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