Skip to content

CLN: Merge scripts/ and ci/ directories #31038

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

Closed
datapythonista opened this issue Jan 15, 2020 · 2 comments
Closed

CLN: Merge scripts/ and ci/ directories #31038

datapythonista opened this issue Jan 15, 2020 · 2 comments
Labels
Clean Closing Candidate May be closeable, needs more eyeballs Needs Discussion Requires discussion from core team before further action

Comments

@datapythonista
Copy link
Member

datapythonista commented Jan 15, 2020

While conceptually it can make sense to separate the scripts that are mostly called from the CI, than the ones that are not, I think in practice the division between scripts/ and ci/ directories is rather arbitrary.

An example is scripts/validate_docstrings.py. We use it from the CI, but we use it locally too to validate a single function. We have also scripts in ci/ that are currently not called from the CI (not sure if they are still useful, will create a separate issue to discuss): ci/check_git_tags.sh, ci/check_cache.sh and ci/travis_process_gbq_encryption.sh

Personally, I think a different structure would make things clearer. An idea would be the next:

dev/release/  <- scripts used for the release
dev/validation/  <- scripts used for validation (docstrings, code checks...)
dev/deps/  <- `ci/deps`
dev/config/  <- `ci/azure`
dev/misc/  <- everything else (may be a specific directory for testing stuff?)

@jbrockmendel I think we discussed about this some time ago, and you didn't like the proposal then. Does this make sense as proposed here? Any idea if you don't like this proposal, but you agree the current structure is confusing?

@datapythonista datapythonista added Clean Needs Discussion Requires discussion from core team before further action labels Jan 15, 2020
@jbrockmendel
Copy link
Member

No strong opinion here. Misc thoughts

  • The scripts/ have a tendency to become obsolete without anyone noticing, so if your proposal makes that easier to prevent, then that's a plus.
  • A lot of work (yours!) has gone into the scripts and the CI config. Last time I tried to configure Travis/Azure for a new project it was a PITA. To the extent that we can make parts of this copy/pasteable by the community, I think that'd be great. (e.g. upstreaming the docstring validation is really positive)
  • The people who work on these the most (you in this case) are most affected by their layout, so should get a strong say in how they are organized.

@jbrockmendel jbrockmendel added the Closing Candidate May be closeable, needs more eyeballs label Sep 24, 2020
@datapythonista
Copy link
Member Author

Probably worth closing this, this is partly outdated, and I don't think there is interest in merging ci/ and scripts/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Closing Candidate May be closeable, needs more eyeballs Needs Discussion Requires discussion from core team before further action
Projects
None yet
Development

No branches or pull requests

2 participants