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

Switch to explicit Turborepo config #2150

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brawaru
Copy link
Contributor

@brawaru brawaru commented Aug 11, 2024

This changes Turborepo config in the workspace root to be explicit.
Explicity in this case means that all tasks are defined manually and all
inputs are also specified manually.

While that might seem redundant, this allows to orchestrate and adjust
tasks more carefully - ignoring some files while taking other files into
account, only hashing and providing project-specific variables,
explicitly reviewing and specifying dependencies. That might come in
handy in the future.

There's an alternative way to approach this, which is to have every
package contain "turbo.json" file, however, for now the flat approach
has been chosen as it seems to be the most cleanest and easiest to
review.

@brawaru brawaru force-pushed the chore/explicit-turborepo-config branch 3 times, most recently from 6d214aa to 7d8ba50 Compare August 15, 2024 12:34
Copy link
Contributor

@Norbiros Norbiros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that you are defining a lot of identical test, lint and fix tasks? Maybe you could remove them and move their's inputs to global ones instead?

turbo.json Show resolved Hide resolved
@brawaru
Copy link
Contributor Author

brawaru commented Aug 18, 2024

I don't like that you are defining a lot of identical test, lint and fix tasks? Maybe you could remove them and move their's inputs to global ones instead?

This is the exact opposite what this PR tries to achieve. It tries to strictly and explicitly describe each task inputs, outputs, dependencies and persistence to avoid any clashes.

Unfortunately, Turborepo doesn't seem to have a concept of default (template) task options. It only has:

  • global task options: which is as it was before, where you define something like lint and this configures options for all tasks called lint except for those that explicitly defined (like below)
  • and single task options, which completely overrides global tasks options, meaning if you define @modrinth/app#lint, it will not inherit any options from lint (for better or for worse)

So it's easier to just explicitly to define everything in case any of the tasks is going to have an exception to its options. It's also much better to see what's going on at a quick glance, without needing to know how Turborepo works under the hood. It's going to end up in some duplication, but that's not a big deal, IMO. Mindless DRY is a terrible concept anyway.

@Norbiros
Copy link
Contributor

This is the exact opposite what this PR tries to achieve. It tries to strictly and explicitly describe each task inputs, outputs, dependencies and persistence to avoid any clashes.

I'm not a fan of this change... I think we can override tasks in the future if necessary, so we don't need to specify identical configurations right now. But I do like that you improved and specified some configurations (mainly build).

@brawaru brawaru force-pushed the chore/explicit-turborepo-config branch from 7d8ba50 to 8fba682 Compare August 19, 2024 14:36
This changes Turborepo config in the workspace root to be explicit.
Explicity in this case means that all tasks are defined manually and all
inputs are also specified manually.

While that might seem redundant, this allows to orchestrate and adjust
tasks more carefully - ignoring some files while taking other files into
account, only hashing and providing project-specific variables,
explicitly reviewing and specifying dependencies. That might come in
handy in the future.

There's an alternative way to approach this, which is to have every
package contain "turbo.json" file, however, for now the flat approach
has been chosen as it seems to be the most cleanest and easiest to
review.
@brawaru brawaru force-pushed the chore/explicit-turborepo-config branch from 8fba682 to ce57dbe Compare August 19, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants