-
Notifications
You must be signed in to change notification settings - Fork 37
[CHORE] update ruff config on remaining files that fail ruff check, update .gitignore to not track personal copilot agent data, local build folders, and vim swap files #900
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
base: main
Are you sure you want to change the base?
Conversation
…pdate .gitignore to not track personal copilot agent data, local build folders, and vim swap files
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #900 +/- ##
=======================================
Coverage 72.69% 72.69%
=======================================
Files 108 108
Lines 10069 10069
Branches 880 880
=======================================
Hits 7320 7320
Misses 2568 2568
Partials 181 181
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
| *.snap | ||
| *.swp | ||
|
|
||
| .github/copilot-instructions.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the recommendation is to not gitignore it, so it is consistent between different developer environments/clones. However, we can ignore it until we decide on a standard copilot-instructions.md
@rene-oromtz @ajzobro do you agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, agree on ignoring for now. If we were to include a standard, maybe we should define a AGENTS.md soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, especially with all the agentic PRs or even to help with some of the chores that devs might do with an agent locally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be formatted. It is a vendored library from a charm (grafana_agent). We should not be editing it.
If Ruff is giving errors or warnings, it should be ignored in a pyproject.toml or ruff.toml configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also notice that while playing with just. I think it was not properly set as ignore unless you run it in a specific way... I'll open a PR to address this, it was drafted on my personal computer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I'm not sure how I feel about formatting the docs/ directory with Ruff. This is what we get from the Canonical Sphinx starter pack. I would think we shouldn't format it too much to make sure we can compare it with the template when there are updates, for example.
@tang-mm what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The starter pack doesn’t include specific formatting rules, so it’s really up to the individual project. I'd lean toward extending the project-level rules to docs/ scripts to keep further local changes consistent (e.g. conf.py currently has a mixed usage of single- and double-quotes 😅 )
make sure we can compare it with the template when there are updates
If we need to compare against the upstream template later, we can just run a quick ruff format on the template first to get a clean diff
|
Honestly feel free to axe this too, it was a pretty quick attempt at just cleaning up things that looked like potential problems later (since we are linting with Ruff formatting now and that blocked my earlier PRs on the maas2 connector). So I'm perfectly happy to reject this and deal with it all later 😸 |
|
Or, if you'd prefer, I can drop the format changes and just PR the .gitignore update? Then you all and @tang-mm can decide on whether or not to use ruff on the docs directory or not... |
…
Description
TSIA - discovered while working on #880 that there are a few files that fail the ruff check, so I did a quick reformat using ruff. Also updated .gitignore to not track copilot agent instructions, VIM swap files, and local build artifact folder.
Resolved issues
Resolves CERTTF-823
Documentation
NA
Web service API changes
NA
Tests
Unit tests checked