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

Add code analysis hooks #20

Merged
merged 5 commits into from
Aug 6, 2024
Merged

Add code analysis hooks #20

merged 5 commits into from
Aug 6, 2024

Conversation

dyastremsky
Copy link
Contributor

Migrate precommit, CodeQL, and unit test hooks to Perf Analyzer repo. We may want to add rules to require these to pass before merging. We had those in the client repo.

@dyastremsky dyastremsky self-assigned this Jul 30, 2024
@dyastremsky dyastremsky changed the title Add precommit hooks Add code analysis hooks Jul 30, 2024
Fix configs

Fix configs
@debermudez
Copy link
Contributor

We may want to add rules to require these to pass before merging. We had those in the client repo.

Can we add those rules?

@fpetrini15
Copy link
Collaborator

Can we add those rules?

@debermudez Yes, I'm working with Meenakshi to get those rules in this place for this repo as well.

@dyastremsky
Copy link
Contributor Author

It looks like this is ready to merge aside from getting advanced security enabled for CodeQL to work, similar to our other repos. Francesco does not have permissions. Do @mc-nv or @dzier?

@fpetrini15
Copy link
Collaborator

@nvda-mesharma any word on the security rule here? Can we merge anyways? These changes must be in place before code freeze in order to align with the doc changes I have.

@dyastremsky
Copy link
Contributor Author

dyastremsky commented Aug 6, 2024

@nvda-mesharma any word on the security rule here? Can we merge anyways? These changes must be in place before code freeze in order to align with the doc changes I have.

@nvda-mesharma We'd really like to get this merged in. If we need to, we can merge it with CodeQL failing for now, but ideally we'd get the automated checks here to match those as our other repos. It says advanced security needs to be enabled (perhaps it just needs to be open-sourced? we didn't need to do anything extra for our other repos, unless there is a setting difference here).

CC: @ganeshku1

@dyastremsky
Copy link
Contributor Author

Based on offline discussion, we are merging this. CodeQL should work once the repo is open-sourced.

@fpetrini15 fpetrini15 merged commit fe5466a into main Aug 6, 2024
3 of 4 checks passed
@fpetrini15 fpetrini15 deleted the dyas-precommit branch August 6, 2024 19:49
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.

3 participants