Skip to content

CLOUDP-301527 - add precommit and apply #642

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.

Sign up for GitHub

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

Jump to bottom
Open
nammn wants to merge 3 commits into
base: master
Choose a base branch
from
Open

CLOUDP-301527 - add precommit and apply #642

nammn wants to merge 3 commits into from
+652 −1,049

Conversation

@nammn
Copy link
Collaborator

@nammn nammn commented Dec 10, 2025 •
edited
Loading

Summary

  • introducing pre-commit and migrating our self written pre-commit hook to it
  • introducing common pre-commit hooks
  • i changed isort to use 120 to match what black does, apparently we didn't before
  • changed snippet validator to skip git ignored folders
  • why: we decided to migrate to a common used tool for better support, composability and maintenance

core changes (out of all files): can be seen in the first commit, all the subsequent commits are fixing those

Next

  • activate file and whitespace linter in the next pr which will also change tons of files

Proof of Work

  • green ci: patch
  • red ci when something is wrong from the first commit: patch

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
  • Have you checked whether your jira ticket required DOCSP changes?
  • Have you added changelog file?

@nammn nammn added the skip-changelog Use this label in Pull Request to not require new changelog entry file label Dec 10, 2025
@nammn nammn force-pushed the pre-commit branch 2 times, most recently from d3ddcb8 to 53ed895 Compare December 11, 2025 07:25
nammn added a commit that referenced this pull request Dec 11, 2025
@nammn @claude
CLOUDP-301527: Migrate to pre-commit framework (core files only)
ae5e266 
This branch contains only the core implementation files for review.
The full PR (#642) includes additional auto-formatted files.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
nammn added a commit that referenced this pull request Dec 11, 2025
@nammn
CLOUDP-301527: Migrate to pre-commit framework (core files only)
1dcf09b 
This branch contains only the core implementation files for review.
The full PR (#642) includes additional auto-formatted files.
@nammn nammn force-pushed the pre-commit branch 2 times, most recently from 922b4cd to c3113c3 Compare December 11, 2025 09:13
@github-actions
Copy link

github-actions bot commented Dec 11, 2025 •
edited
Loading

⚠️ (this preview might not be accurate if the PR is not rebased on current master branch)

MCK 1.6.2 Release Notes

nammn
nammn commented Dec 11, 2025
View reviewed changes
scripts/dev/generate_files.sh
@@ -0,0 +1,213 @@
#!/usr/bin/env bash
Copy link
Collaborator Author

@nammn nammn Dec 11, 2025

Choose a reason for hiding this comment

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

thyis is mostly cp and paste from pre-commit

Copy link
Collaborator Author

@nammn nammn Dec 11, 2025

Choose a reason for hiding this comment

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

in later pr i plan to enable newlines and trailing whitespace fixer, but then we will need to ensure that all generated files follow this - otherwise, pre-commit will fail if pre-commit needs to fix it

nammn
nammn commented Dec 11, 2025
View reviewed changes
scripts/dev/generate_files.sh Outdated
source scripts/dev/set_env_context.sh
source scripts/funcs/printing

# Strip trailing whitespace from files to ensure clean generation output
Copy link
Collaborator Author

@nammn nammn Dec 11, 2025

Choose a reason for hiding this comment

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

if we don't do this, then we generate and then pre-commit runs and fixes them and then it fails as it changed something

@nammn nammn marked this pull request as ready for review December 11, 2025 11:48
@nammn nammn requested review from a team and vinilage as code owners December 11, 2025 11:48
@nammn nammn force-pushed the pre-commit branch 4 times, most recently from f4fde10 to 1c4f4be Compare December 11, 2025 12:28
nammn
nammn commented Dec 11, 2025
View reviewed changes
@nammn nammn marked this pull request as draft December 11, 2025 12:42
@nammn nammn force-pushed the pre-commit branch 4 times, most recently from 224f8a2 to d68a102 Compare December 11, 2025 14:26
@nammn nammn marked this pull request as ready for review December 12, 2025 12:23
@nammn nammn requested a review from m1kola December 12, 2025 12:28
@nammn nammn force-pushed the pre-commit branch 4 times, most recently from be1c93a to 0b8d1f6 Compare December 12, 2025 13:45
@nammn nammn marked this pull request as draft December 12, 2025 14:00
@nammn nammn marked this pull request as ready for review December 12, 2025 14:27
nammn
nammn commented Dec 15, 2025
View reviewed changes
nammn added 2 commits December 18, 2025 15:54
@nammn
CLOUDP-301527: Migrate from custom pre-commit to official pre-commit …
e25aebf 
…framework

- Add .pre-commit-config.yaml with hooks for golangci-lint, shellcheck, black, isort, etc.
- Add generate_files.sh and regenerate_multicluster_rbac.sh scripts
- Update Makefile, pyproject.toml, requirements.txt for pre-commit integration
- Update evergreen scripts for pre-commit checks
- Run pre-commit generation jobs in parallel
@nammn
chore: Apply pre-commit hooks (black, isort, shellcheck)
c14c030 
- Auto-format Python files with black and isort
- Apply shellcheck fixes to shell scripts
@evergreen-ci-prod
Copy link

evergreen-ci-prod bot commented Dec 18, 2025

There is an existing patch(es) for this commit SHA:

Please note that the status that is posted is not in the context of this PR but rather the (latest) existing patch and that may affect some tests that may depend on the particular PR. If your tests do not rely on any PR-specific values (like base or head branch name) then your tests will report the same status. If you would like a patch to run in the context of this PR and abort the other(s), comment 'evergreen retry'.

nammn
nammn commented Dec 18, 2025
View reviewed changes
.pre-commit-config.yaml

default_stages: [pre-commit]

repos:
Copy link
Collaborator Author

@nammn nammn Dec 18, 2025

Choose a reason for hiding this comment

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

this is mostly suggested and generated by llms

nammn
nammn commented Dec 18, 2025
View reviewed changes
.pre-commit-config.yaml
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
hooks:
# TODO: Enable these hooks in a follow-up PR - they modify 200+ files
Copy link
Collaborator Author

@nammn nammn Dec 18, 2025

Choose a reason for hiding this comment

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

commented them out for alter prs, the same for yamllinter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vinilage vinilage Awaiting requested review from vinilage

@fealebenpae fealebenpae Awaiting requested review from fealebenpae fealebenpae is a code owner automatically assigned from mongodb/kubernetes-hosted

@anandsyncs anandsyncs Awaiting requested review from anandsyncs anandsyncs is a code owner automatically assigned from mongodb/kubernetes-hosted

@m1kola m1kola Awaiting requested review from m1kola

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

skip-changelog Use this label in Pull Request to not require new changelog entry file

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nammn