Fix pre-commit and linter versions to avoid surprise #1100
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To avoid surprising results on GitHub Actions, e.g. a new release of pre-commit starts to reject all PRs, fix it to the latest version, 3.7.0. This PR also fixes ruff and mypy versions in pyproject.toml since we very likely don't really need latest upgrades from linters, and upgrades can always bring surprise.
auto-merge was automatically disabled
April 14, 2024 18:32
Head branch was pushed to by a user without write access
|
@neubig Thanks for the quick review. Looks like I need you to sign-off once more since I pushed an one-liner after your approval. |
enyst
approved these changes
Apr 14, 2024
li-boxuan
added a commit
to li-boxuan/OpenDevin
that referenced
this pull request
Apr 15, 2024
This has been a headache for a long time, and we had All-Hands-AI#1071 and All-Hands-AI#1100 with the hope to fix the inconsistent behaviour across linters and environments. However, we recently found out that `double-quote-string-fixer` plugin in pre-commit-hook has inconsistent behaviour on python 3.11 and 3.12. This is sad because while this plugin enforces single quote behaviour with 3.11, it doesn't always enforce so with 3.12. Specifically, with f"str" syntax, this plugin allows both single quotes with double quotes with python 3.12. The problem is, some developers have black linter installed/integrated with their IDE, which is probably the most popular linter in python world (ranked by GitHub stars). This linter insists on always using double quotes. Now we have `black` and `double-quote-string-fixer` fight each other (iff the developer uses python 3.12) for some quotes (f"str" syntax). After a lot of research, I couldn't find a way to enforce single quote behaviour without introducing a new dependency, flake8. I believe it's better off introducing the more popular black if we have to introduce a new linter. Since black and autopep8 sometimes fight each other, and they mostly overlap, I further remove autopep8. The unfortunate consequence of this PR is that I had to revert all single quotes back to double quotes. This might cause some inconvenience to existing PRs as they have to resolve conflicts, but I believe the headache will be gone soon. That being said, I am open to abandon this PR if anyone has a better idea to solve the headache.
li-boxuan
added a commit
to li-boxuan/OpenDevin
that referenced
this pull request
Apr 15, 2024
This has been a headache for a long time, and we had All-Hands-AI#1071 and All-Hands-AI#1100 with the hope to fix the inconsistent behaviour across linters and environments. However, we recently found out that `double-quote-string-fixer` plugin in pre-commit-hook has inconsistent behaviour on python 3.11 and 3.12. This is sad because while this plugin enforces single quote behaviour with 3.11, it doesn't always enforce so with 3.12. Specifically, with f"str" syntax, this plugin allows both single quotes with double quotes with python 3.12. The problem is, some developers have black linter installed/integrated with their IDE, which is probably the most popular linter in python world (ranked by GitHub stars). This linter insists on always using double quotes. Now we have `black` and `double-quote-string-fixer` fight each other (iff the developer uses python 3.12) for some quotes (f"str" syntax). After a lot of research, I couldn't find a way to enforce single quote behaviour without introducing a new dependency, flake8. I believe it's better off introducing the more popular black if we have to introduce a new linter. Since black and autopep8 sometimes fight each other, and they mostly overlap, I further remove autopep8. The unfortunate consequence of this PR is that I had to revert all single quotes back to double quotes. This might cause some inconvenience to existing PRs as they have to resolve conflicts, but I believe the headache will be gone soon. That being said, I am open to abandon this PR if anyone has a better idea to solve the headache.
li-boxuan
added a commit
to li-boxuan/OpenDevin
that referenced
this pull request
Apr 15, 2024
…and All-Hands-AI#1100 with the hope to fix the inconsistent behaviour across linters and environments. However, we recently found out that double-quote-string-fixer plugin in pre-commit-hook has inconsistent behaviour on python 3.11 and 3.12. See discussion here. This is sad because while this plugin enforces single quote behaviour with 3.11, it doesn't always enforce so with 3.12. Specifically, with fstr syntax, this plugin allows both single quotes and double quotes with python 3.12. The problem is, some developers have black linter installed/integrated with their IDE, which is probably the most popular linter in python world (ranked by GitHub stars). This linter insists on always using double quotes. Now we have black and double-quote-string-fixer fight each other (iff the developer uses python 3.12) for some quotes (fstr syntax). After a lot of research, I couldn't find a way to enforce single quote behaviour without introducing a new dependency, flake8, together with a plugin for it to enforce quotes' behavior. I believe it's better off introducing the more popular black if we have to introduce a new linter. Since black and autopep8 sometimes fight each other, and they mostly overlap, I further remove autopep8. The unfortunate consequence of this PR is that I had to revert all single quotes back to double quotes. This might cause some inconvenience to existing PRs as they have to resolve conflicts, but I believe the headache will be gone soon. That being said, I am open to abandon this PR if anyone has a better idea to solve the headache.
foragerr
pushed a commit
that referenced
this pull request
Apr 17, 2024
* This has been a headache for a long time, and we had #1071 and #1100 with the hope to fix the inconsistent behaviour across linters and environments. However, we recently found out that double-quote-string-fixer plugin in pre-commit-hook has inconsistent behaviour on python 3.11 and 3.12. See discussion here. This is sad because while this plugin enforces single quote behaviour with 3.11, it doesn't always enforce so with 3.12. Specifically, with fstr syntax, this plugin allows both single quotes and double quotes with python 3.12. The problem is, some developers have black linter installed/integrated with their IDE, which is probably the most popular linter in python world (ranked by GitHub stars). This linter insists on always using double quotes. Now we have black and double-quote-string-fixer fight each other (iff the developer uses python 3.12) for some quotes (fstr syntax). After a lot of research, I couldn't find a way to enforce single quote behaviour without introducing a new dependency, flake8, together with a plugin for it to enforce quotes' behavior. I believe it's better off introducing the more popular black if we have to introduce a new linter. Since black and autopep8 sometimes fight each other, and they mostly overlap, I further remove autopep8. The unfortunate consequence of this PR is that I had to revert all single quotes back to double quotes. This might cause some inconvenience to existing PRs as they have to resolve conflicts, but I believe the headache will be gone soon. That being said, I am open to abandon this PR if anyone has a better idea to solve the headache. * Remove black * Prevent black from changing quotes * Use flake8 to enforce single quotes * Fix quotes in config.py * Add back autopep8 * Add make lint to run linters
lehcode
added a commit
to lehcode/oppendevin
that referenced
this pull request
Apr 17, 2024
* lint-frontend Files.tsx (All-Hands-AI#1089) for lint-frontend exhaustive deps Co-authored-by: Robert Brennan <accounts@rbren.io> * Add build-frontend to build (All-Hands-AI#1137) * build: fix workspace variable name in dev setup (All-Hands-AI#1138) * Revamp docker build process (All-Hands-AI#1121) * refactor docker building * change to buildx * disable branch filter * disable tags * matrix for building * fix branch filter * rename workflow * sanitize ref name * fix sanitization * fix source command * fix source command * add push arg * enable for all branches * logs * empty commit * try freeing disk space * try disk clean again * try alpine * Update ghcr.yml * Update ghcr.yml * move checkout * ignore .git * add disk space debug * add df h to build script * remove pull * try another failure bypass * remove maximize build space step * remove df -h debug * add no-root * multi-stage python build * add ssh * update readme * remove references to config.toml * fix(): build out opendevin modal component (All-Hands-AI#1141) * corrected port (All-Hands-AI#1159) * fix All-Hands-AI#1028, /select-file api call on deleted file in Code Editor caused Error (All-Hands-AI#1158) * fix: /select-file on deleted file exception, detail: All-Hands-AI#1028 * fix: lint. * fix: lint. --------- Co-authored-by: aaren.xzh <aaren.xzh@antfin.com> * Remove custom signal handlers (All-Hands-AI#1153) * (fix) OpenDevin works on OpenDevin issues (All-Hands-AI#1149) * Use SANDBOX_TYPE=exec, use docker image * Use updated image * fix: fix multiple frontend warnings (All-Hands-AI#1143) * test(frontend): add unit tests (All-Hands-AI#1076) * test(frontend): add unit tests for getCachedConfig * test(frontend): add unit tests for getCachedConfig * add unit test for the useTypingEffect hook * add unit test for the useInputComposition hook * create unit test for auth service * remove outdated and failing component test * create unit test for session service * break down saveSettings into smaller functions for testability and create unit test for new mergeAndUpdateSettings --------- Co-authored-by: Robert Brennan <accounts@rbren.io> * Logging additions and fixes (All-Hands-AI#1139) * Refactor print_to_color into a color formatter misc fixes catch ValueErrors and others from Router initialization add default methods * Tweak console log formatting, clean up after rebasing exceptions out * Fix prompts/responses * clean up * keep regular colors when no msg_type * fix filename * handle file log first * happy mypy * ok, mypy --------- Co-authored-by: Robert Brennan <accounts@rbren.io> * fix: make typing chat bubble adhere to max width (All-Hands-AI#1142) * fix: fix delete messages endpoint url (All-Hands-AI#1140) Co-authored-by: Robert Brennan <accounts@rbren.io> * Run docker publish on tags (All-Hands-AI#1162) * run ghcr on tags * Update .github/workflows/ghcr.yml Co-authored-by: RaGe <foragerr@users.noreply.github.com> --------- Co-authored-by: RaGe <foragerr@users.noreply.github.com> * Document LLM prompt and response logging. (All-Hands-AI#1166) * Refactor actions a bit (All-Hands-AI#1165) * refactor to action_manager * make all actions awaitable * move task logic into tasks * Update opendevin/action/tasks.py * feat: Improve docker compose services integration * feat: Add ollama, support, memGPT services * feat: Configure OpenDevin UI container * feat: Docker services * hotfix: Add OS locales * Merge develop into clean-install * Add ollama, support, memGPT services * feat: Docker services * hotfix: Backend app service dependencies fix under Conda hotfix: Frontend integration hotfix: Restore useTranslation() * feat: Add API startup script hotfix: Integration fix * feat: Add FastAPI server and Vite dev server logging for debug and live modes * hotfix: Frontend and API integration. Build improvements. * Add ollama, support, memGPT services * feat: Docker services hotfix: Add OS locales hotfix: Fix start configuration hotfix: Temp fix of build errors feat: Configure OpenDevin UI container hotfix: Run OpenDevin application container chore: Update README feat: Docker Configuration for backend services feat: Ubuntu 20.04 images with CUDA and Miniconda3 refactor: Update docker environment configuration feat: Decouple UI and Python app into services. Co-authored-by: Jim Su <jimsu@protonmail.com> * hotfix: Frontend integration * hotfix: Update Miniconda source package and SHA256 hash * hotfix: Make ESLint 'no-duplicate-imports' to report warnings instead of errors * chore: Arrange '.env' and '.env.dist' files * chore: Rename 'nginx.conf' to 'nginx.conf.dist' * chore: Update Docker services configuration * feat: Add developer.Dockerfile to Devin web UI Docker config. Move Dockerfile to end-uder.Dockerfile * feat: Update NGINX configuration with HTTP/HTTPS server config * feat: Update Devin API host and port * chore: Remove unnecessary docker-compose.yml file version tag * chore: Revert 'packageManager' to pnpm * feat: Add frontend port EXPOSE * feat: Add API ports EXPOSE directives * hotfix: Path to 'env_debug' script * feat: Add developer.Dockerfile * chore: Synchronize with OpenDevin main branch * replace Jest with Vitest as the frontend test runner (All-Hands-AI#1163) * Fix python linter inconsistent behaviour with quotes (All-Hands-AI#1112) * This has been a headache for a long time, and we had All-Hands-AI#1071 and All-Hands-AI#1100 with the hope to fix the inconsistent behaviour across linters and environments. However, we recently found out that double-quote-string-fixer plugin in pre-commit-hook has inconsistent behaviour on python 3.11 and 3.12. See discussion here. This is sad because while this plugin enforces single quote behaviour with 3.11, it doesn't always enforce so with 3.12. Specifically, with fstr syntax, this plugin allows both single quotes and double quotes with python 3.12. The problem is, some developers have black linter installed/integrated with their IDE, which is probably the most popular linter in python world (ranked by GitHub stars). This linter insists on always using double quotes. Now we have black and double-quote-string-fixer fight each other (iff the developer uses python 3.12) for some quotes (fstr syntax). After a lot of research, I couldn't find a way to enforce single quote behaviour without introducing a new dependency, flake8, together with a plugin for it to enforce quotes' behavior. I believe it's better off introducing the more popular black if we have to introduce a new linter. Since black and autopep8 sometimes fight each other, and they mostly overlap, I further remove autopep8. The unfortunate consequence of this PR is that I had to revert all single quotes back to double quotes. This might cause some inconvenience to existing PRs as they have to resolve conflicts, but I believe the headache will be gone soon. That being said, I am open to abandon this PR if anyone has a better idea to solve the headache. * Remove black * Prevent black from changing quotes * Use flake8 to enforce single quotes * Fix quotes in config.py * Add back autopep8 * Add make lint to run linters * hotfix: PR fix * Hotfix/clean install (#21) * hotfix: Added missing variables. Removed .env from repo --------- Co-authored-by: 808vita <97225946+808vita@users.noreply.github.com> Co-authored-by: Robert Brennan <accounts@rbren.io> Co-authored-by: RaGe <foragerr@users.noreply.github.com> Co-authored-by: Alex Bäuerle <alex@a13x.io> Co-authored-by: Akki <akhilvc10@gmail.com> Co-authored-by: மனோஜ்குமார் பழனிச்சாமி <smartmanoj42857@gmail.com> Co-authored-by: Xia Zhenhua <zhhuaxia@gmail.com> Co-authored-by: aaren.xzh <aaren.xzh@antfin.com> Co-authored-by: Boxuan Li <liboxuan@connect.hku.hk> Co-authored-by: sp.wack <83104063+amanape@users.noreply.github.com> Co-authored-by: Engel Nyst <enyst@users.noreply.github.com> Co-authored-by: Jim Su <jimsu@protonmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have a lot of headache with linting this project. Although #1071 made a first attempt to unify the behavior of local linting and CI linting, it still has the following limitations:
To avoid surprising results on GitHub Actions, e.g. a new release of pre-commit starts to reject all PRs, fix it to the latest version, 3.7.0, in both pyproject.toml and lint.yml. This PR also fixes ruff and mypy versions in pyproject.toml since we very likely don't really need latest upgrades from linters.