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

Update .pre-commit-config.yaml and Implement Suggestions #12

Merged
merged 21 commits into from
Oct 31, 2024

Conversation

nv-rliu
Copy link
Contributor

@nv-rliu nv-rliu commented Oct 30, 2024

Closes #11

Notes for Reviewers

This PR was mainly designed to consolidate (previously) lint.yaml into pre-commit-hooks.yaml. I tried to preserve all the hooks from lint.yaml and remove hooks/directories/etc that are no longer needed. Feel free to let me know if some of these checks are redundant or seem incorrect.

Proposed Changes

  • Add additional script to ignore in .flake8 config
  • Merged previous lint.yaml file into .pre-commit-config.yaml
    • Updated hooks to latest version
  • Redirect pyproject_dir in dependencies.yaml after getting rid of the old python/nx-cugraph dir
  • Remove a bunch of old cugraph docs
  • Update pyproject.toml using pre-commit

@nv-rliu nv-rliu added breaking Introduces a breaking change improvement Improves an existing functionality and removed benchmarks labels Oct 30, 2024
@nv-rliu nv-rliu marked this pull request as ready for review October 30, 2024 02:16
@nv-rliu nv-rliu requested review from a team as code owners October 30, 2024 02:16
@nv-rliu nv-rliu requested review from jameslamb and removed request for a team October 30, 2024 02:16
Copy link
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

I left some suggestions for your consideration.

Other notes:

  • please add CI here in this PR just running pre-commit (similar to introduce minimal CI for PRs cugraph-gnn#56). That'd make us more confident in these changes
  • There are many references in this diff to "cugraph" which really should be "nx-cugraph". I suspect there are more that are in the repo but didn't show up in the diff... those should be fixed (can be a follow-up PR).

.pre-commit-config.yaml Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
docs/nx-cugraph/source/conf.py Outdated Show resolved Hide resolved
docs/nx-cugraph/source/conf.py Outdated Show resolved Hide resolved
@nv-rliu nv-rliu requested a review from jameslamb October 30, 2024 18:15
@github-actions github-actions bot added the ci label Oct 30, 2024
@nv-rliu nv-rliu merged commit 3e09b1c into rapidsai:branch-24.12 Oct 31, 2024
6 checks passed
@nv-rliu nv-rliu deleted the 24.12-pre-commit branch October 31, 2024 02:39
@nv-rliu nv-rliu restored the 24.12-pre-commit branch October 31, 2024 02:39
@nv-rliu nv-rliu deleted the 24.12-pre-commit branch November 7, 2024 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmarks breaking Introduces a breaking change ci improvement Improves an existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA]: Merge .pre-commit-config.yaml with lint.yaml
2 participants