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

Task semantics behaviour #276

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Task semantics behaviour #276

wants to merge 16 commits into from

Conversation

zuzuleinen
Copy link
Contributor

@zuzuleinen zuzuleinen commented Nov 30, 2022

This PR implemented all scenarios from #229 under Behavior based on those changes section.

More specific:

  • input is mandatory on build tasks except when task has rebuild:always or is a compound task(has only dependsOn node on it)
  • target and rebuild: always are not allowed on the same task. They are incompatible now.
  • a new state was added StateNoRebuildRequired which is similar StateCached. The differennce is that the reason for StateCached is because the targed was fetched from cache while the reason for StateNoRebuildRequired was that a rebuild was not necessary but there was no target to fetch from cache.

Tests for functionality are added in test/e2e/tasksemantics. The are all green, however there are still tests that needs fixing.

@Equanox
Copy link
Member

Equanox commented Dec 1, 2022

  • The task status no-rebuild feels not right. E.g. npm run lint or go fmt doesn't build anything.
  • Shall we avoid showing a state on "compound" tasks? It's a bit useless.
  • I see some difficulties during testing when forbidding rebuild: always together with target: (aka redundant targets). Shall we make the behavior adjustable? Adding a mode on which we allow Bob redundant targets, WithAllowRedundantTarget()

@zuzuleinen zuzuleinen marked this pull request as ready for review December 2, 2022 13:51
@zuzuleinen zuzuleinen requested a review from Equanox December 2, 2022 13:51
bob/aggregate.go Outdated Show resolved Hide resolved
@zuzuleinen zuzuleinen requested a review from Equanox December 7, 2022 08:52
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.

2 participants