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

Refactor/rename directory structure #9940

Closed
Tracked by #19289
rarkins opened this issue May 10, 2021 · 10 comments
Closed
Tracked by #19289

Refactor/rename directory structure #9940

rarkins opened this issue May 10, 2021 · 10 comments
Assignees
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:in-progress Someone is working on implementation type:refactor Refactoring or improving of existing code

Comments

@rarkins
Copy link
Collaborator

rarkins commented May 10, 2021

I'm thinking of ways to restructure our source code inside lib/ for easier understanding. Ideas include:

  • Moving datasource, manager, platform and versioning under a new modules/ subdirectory
  • Restructuring workers so that the source code is sorted closer to execution order

After these changes, it would leave the following under lib/: config, constants, logger, types and util.

@rarkins rarkins added priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:refactor Refactoring or improving of existing code status:requirements Full requirements are not yet known, so implementation should not be started labels May 10, 2021
@viceice
Copy link
Member

viceice commented May 10, 2021

This would help for docs too, because currently we need to change the levels, which should hopefully make cross links easier.

@rarkins rarkins added the breaking Breaking change, requires major version bump label Feb 25, 2022
@rarkins
Copy link
Collaborator Author

rarkins commented Feb 25, 2022

I want to do this on the next major release:

lib/
  modules/
    datasource/
    manager/
    platform/
    versioning/
  workers/
    global/
      config/
      init/
      run/
      teardown/
    repository/
      config/
      extract/
      lookup/
      update/
        branch/
        pr/

From experimenting on modules today, I found that the follow-up tasks included:

  • Updating the ts strict config
  • Looking for jest.mock('../ and adding an extra ../ to it
  • Updating snapshots

@rarkins
Copy link
Collaborator Author

rarkins commented Feb 26, 2022

After modules moving, replace:

  • jest.mock\('(\.\./(util|config|logger)) in lib/modules with jest.mock('../$1
  • \.\./(datasource|manager|platform|versioning) in lib/(config|constants|util|workers) with ../modules/$1
  • \.\./(config|constants|logger|util|workers|test|types) in lib/modules/ with ../../$1
  • more..

@viceice
Copy link
Member

viceice commented Feb 26, 2022

VSCode can auto update imports when moving via drag'n'drop.

@rarkins
Copy link
Collaborator Author

rarkins commented Feb 26, 2022

I know, but for some reason it's not getting all of them. It doesn't catch jest.mock() either

@PhilipAbed
Copy link
Collaborator

@viceice nope, github/ webstorm/ vs code, i checked all, none of them move imports or any hardcoded tests/mocks, doing everything manually with regexes

@rarkins
Copy link
Collaborator Author

rarkins commented Mar 2, 2022

I can confirm that typescript.updateImportsOnFileMove.enabled does work, especially for imports. But:

  • it seems like VSCode can get overloaded and not catch them all, so I recommend small updates at a time (e.g. move one modules at a time)
  • it misses jest.mock() because that's library-specific and not a JS/TS concept

@rarkins
Copy link
Collaborator Author

rarkins commented Mar 2, 2022

Let's do just the potentially "breaking" changes for now:

  • All modules
  • Move branch and pr workers under global worker

@rarkins
Copy link
Collaborator Author

rarkins commented Mar 3, 2022

Some observations:

VSCode indeed does a good job of updating imports. But beware:

  • Make sure you've opened at least one .ts file since restarting, or else the "ESLint server" won't be running
  • After moving a folder, and letting VSCode make the changes, you should wait until the errors/warnings in the bottom bar stabilize and no longer change in count. Only once this is stable can you be free to "Save All" tabs and then Close All

Most of what's left is jest.import() and a few require() statements which were missed, plus replacing any lib/x references to modules to lib/modules/x

This was referenced Mar 3, 2022
@rarkins rarkins removed the breaking Breaking change, requires major version bump label Mar 4, 2022
@rarkins
Copy link
Collaborator Author

rarkins commented Mar 4, 2022

This was mostly implemented in #14490. Remaining parts are no longer "breaking"

@rarkins rarkins added status:in-progress Someone is working on implementation and removed status:requirements Full requirements are not yet known, so implementation should not be started labels Mar 4, 2022
@rarkins rarkins closed this as completed Jun 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:in-progress Someone is working on implementation type:refactor Refactoring or improving of existing code
Projects
None yet
Development

No branches or pull requests

3 participants