Re: [PR] chore(ci): make pre-commit step faster by skipping superset install [superset]
mistercrunch commented on code in PR #27979: URL: https://github.com/apache/superset/pull/27979#discussion_r1561851846 ## .github/workflows/pre-commit.yml: ## @@ -35,6 +37,7 @@ jobs: brew install norwoodj/tap/helm-docs - name: pre-commit run: | + pip install pre-commit Review Comment: Or how about this!? https://github.com/apache/superset/pull/28000 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] chore(ci): make pre-commit step faster by skipping superset install [superset]
mistercrunch commented on code in PR #27979: URL: https://github.com/apache/superset/pull/27979#discussion_r1561821563 ## .github/workflows/pre-commit.yml: ## @@ -35,6 +37,7 @@ jobs: brew install norwoodj/tap/helm-docs - name: pre-commit run: | + pip install pre-commit Review Comment: we can revert and load the whole bundle, not a biggy -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] chore(ci): make pre-commit step faster by skipping superset install [superset]
mistercrunch commented on code in PR #27979: URL: https://github.com/apache/superset/pull/27979#discussion_r1561821992 ## .github/workflows/pre-commit.yml: ## @@ -35,6 +37,7 @@ jobs: brew install norwoodj/tap/helm-docs - name: pre-commit run: | + pip install pre-commit Review Comment: here -> https://github.com/apache/superset/pull/27999 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] chore(ci): make pre-commit step faster by skipping superset install [superset]
villebro commented on code in PR #27979: URL: https://github.com/apache/superset/pull/27979#discussion_r1561785640 ## .github/workflows/pre-commit.yml: ## @@ -35,6 +37,7 @@ jobs: brew install norwoodj/tap/helm-docs - name: pre-commit run: | + pip install pre-commit Review Comment: @john-bodley maybe we should break out the light weight linting packages into a separate requirements file and depend on that in the development one? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] chore(ci): make pre-commit step faster by skipping superset install [superset]
john-bodley commented on code in PR #27979: URL: https://github.com/apache/superset/pull/27979#discussion_r1561366109 ## .github/workflows/pre-commit.yml: ## @@ -35,6 +37,7 @@ jobs: brew install norwoodj/tap/helm-docs - name: pre-commit run: | + pip install pre-commit Review Comment: Sadly @mistercrunch this is not right. We should never run `pip install` without a specific version as it _could_ result in a non-deterministic build when running CI. Instead we should install the frozen requirements in `requirements/development.txt`. Granted this brings in a lot of extra packages, but this was less of an issue when we had more environments including `requirements/integration.[in|txt]` which included only the subset of packages needed to run various CI commands. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] chore(ci): make pre-commit step faster by skipping superset install [superset]
mistercrunch merged PR #27979: URL: https://github.com/apache/superset/pull/27979 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
