You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/04/21 14:24:22 UTC

[GitHub] [airflow] ashb commented on a change in pull request #8487: Add pytest plugins that are useful for optimisations

ashb commented on a change in pull request #8487:
URL: https://github.com/apache/airflow/pull/8487#discussion_r412230961



##########
File path: setup.py
##########
@@ -446,6 +446,9 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
     'pytest',
     'pytest-cov',
     'pytest-instafail',
+    'pytest-rerunfailures',

Review comment:
       > add requirements to setup.py" and "use requirements" PR.
   
   From a git history point-of-view I'd rather that adding the requirement to setup.py and using it was in the same commit, otherwise cherry-picking will be almost impossible to get right (you have to pick the version update, and the change it needs).
   
   Not to mention that makes it rather more complex for contributors to add new modules -- they'd have to make a PR to add the version, wait for tests to pass, get it merged, then make the change they actually wanted. For that reason alone I think this is the wrong approach -- we should add to setup.py and use it in the same PR. And we'll just have to live with the requirements files changing.
   
   Sidenote: my ideal solution isn't possible without rewriting something like pipenv/poetry:
   
   Thinking about how `yarn` handles this: adding a new requirement will only add this single new dep and it's dependencies, to the lock file, and it's only running `yarn update` or similar that would update all the other versions.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org