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

Makefile: make explicit that node_modules is dependency of base #1281

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

matthew-white
Copy link
Member

@matthew-white matthew-white commented Nov 8, 2024

This PR adds node_modules to the list of dependencies of the base task in the Makefile. node_modules was already a dependency, because base depends on node_version, and node_version depends on node_modules. However, I think it'd be clearer for us to include node_modules in the list explicitly. node_modules is conceptually distinct from node_version: even if we got rid of the node_version check, base would still need to run node_modules. In other words, node_modules is not just a sub-dependency of node_version.

We do something similar with node_version and the migrations task. base runs migrations, and migrations runs node_version, yet we still include node_version in the list explicitly.

Before submitting this PR, please make sure you have:

  • run make test and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code from external sources are properly credited in comments or that everything is internally sourced

@matthew-white matthew-white requested a review from ktuite November 8, 2024 20:28
@matthew-white matthew-white merged commit b81f13c into master Nov 8, 2024
7 checks passed
@matthew-white matthew-white deleted the base_node_modules branch November 8, 2024 23:28
alxndrsn pushed a commit to alxndrsn/odk-central-backend that referenced this pull request Nov 9, 2024
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