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

👷 Reorganise scripts directory #2059

Merged
merged 8 commits into from
Mar 21, 2023
Merged

👷 Reorganise scripts directory #2059

merged 8 commits into from
Mar 21, 2023

Conversation

bcaudan
Copy link
Contributor

@bcaudan bcaudan commented Mar 7, 2023

Motivation

Too many files in the scripts directory.

Changes

Introduce new directories to group script files:

  • build
  • release
  • deploy
  • test
  • lib

Testing

ci


I have gone over the contributing documentation.

@bcaudan bcaudan marked this pull request as ready for review March 7, 2023 14:11
@bcaudan bcaudan requested a review from a team as a code owner March 7, 2023 14:11
scripts/build/build-env.js Outdated Show resolved Hide resolved
@amortemousque
Copy link
Contributor

💬 suggestion: We could move deployment-utils in deploy since it's only used in deploy scripts.

It contradicts #2049 (comment) but is align with the chosen strategy discussed in #2053 (comment)

@bcaudan
Copy link
Contributor Author

bcaudan commented Mar 8, 2023

💬 suggestion: We could move deployment-utils in deploy since it's only used in deploy scripts.

It contradicts #2049 (comment) but is align with the chosen strategy discussed in #2053 (comment)

IMO it is interesting to clearly distinguish executables and utility files which is not necessarily the case for source code and utility.
For now, the file name makes it pretty clear that it is related to deploy scripts but:

  • utility files may grow and we could extract some specific logic to other files
  • it could be interesting to have it closer to related scripts

Considered options:

  1. lib directory by domain
lib
  | - utils.js
deploy
  | - script.js
  | - lib
        | -utils.js
  1. directory by domain in lib
lib
  | - utils.js
  | - deploy
         | -utils.js
deploy
  | - script.js

I'd favour the first option, any thoughts?
@amortemousque @BenoitZugmeyer

@BenoitZugmeyer
Copy link
Member

The first option works for me :)

@bcaudan bcaudan merged commit 6c436f1 into main Mar 21, 2023
@bcaudan bcaudan deleted the bcaudan/organize-scripts branch March 21, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants