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 lint check plugin for DN API endpoints #2787

Merged
merged 25 commits into from
Apr 2, 2022
Merged

Conversation

rickyrombo
Copy link
Contributor

@rickyrombo rickyrombo commented Mar 30, 2022

Description

Creates flask_decorator_plugin which is a flake8 plugin to help keep the API well documented and formatted. Note that I also have a PR up with a test for API documentation:

See README for rules and rationales

Also StyleGuide in Notion

It's pretty naive, super opinionated, and definitely not optimized and I'm sure there will be edge cases, but this will help keep the chaos a bit more structured. This also lays the groundwork if there's other rules/plugins we want on our wishlist and we can always enable/disable rules and iterate.

It's also super super niche and definitely makes a lot of assumptions on how the routes are formatted/how the routes are documented based on our existing patterns. If anyone gets into an edge case from one of these, let me know.

Callouts/Qs

  • Do we want to enforce all routes to be literal strings so that the linter can check for route params? Right now if the route is an ast.Name or a function or something it ignores that check.
    • Tabling for now

Tests

Adds some basic test cases. Is itself tested against DN's current clean state.

How will this change be monitored? Are there sufficient logs?

Sending a message in Slack alerting team to ping me if they run into issues.

@rickyrombo rickyrombo requested a review from jowlee March 30, 2022 22:13
@rickyrombo rickyrombo marked this pull request as ready for review March 30, 2022 22:13
@rickyrombo rickyrombo changed the base branch from mjp-api-gen-tests to master April 1, 2022 04:23
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Apr 1, 2022
@rickyrombo rickyrombo merged commit f2030ea into master Apr 2, 2022
@rickyrombo rickyrombo deleted the mjp-api-gen-lint branch April 2, 2022 09:00
theoilie pushed a commit that referenced this pull request Apr 4, 2022
* Add linting flake8 plugin to ensure decorator order and route params in API declarations

* Remove __init__.py

* Revert "Remove __init__.py"

This reverts commit 2bd9b80.

* Modify visitor to make testing easier

* Disable plugin test, rely on visitor test instead

* Move to using local plugin

* Disable rule temporarily

* Fix bugs, add new rule, solve edge cases

* Enable linter

* add default case

* Add tests to vscode test config

* Made plugin more readable

* Move to subfolder

* Move to plugin.py

* Split to several files

* Update setup.cfg to point to plugin

* Make readme

* Fix order config

* Fix README typo

* Fix lint errors

* Use absolute imports

* Set root correctly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants