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

fix: Update esbuild binary fetching logic to look inside of build_dir #550

Merged
merged 4 commits into from
Oct 4, 2023

Conversation

lucashuy
Copy link
Contributor

Issue #, if available:
N/A

Description of changes:
The workflow currently looks at the scratch directory to determine if esbuild is present or not. This is not the case for build in source however, as the Node modules are installed in the source directory, not the scratch directory.

This change updates the path to be build_dir, which is already set to the correct folder from earlier in the workflow.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…y considers build in source/not build in source
@lucashuy lucashuy marked this pull request as ready for review October 4, 2023 16:47
@lucashuy lucashuy requested a review from a team as a code owner October 4, 2023 16:47
@lucashuy lucashuy requested review from hawflau and jfuss October 4, 2023 16:47
@mndeveci
Copy link
Contributor

mndeveci commented Oct 4, 2023

I see how this will work with build in source improvement, but will it still work with existing workflow which builds in scratch directory? cc: @mildaniel

@lucashuy
Copy link
Contributor Author

lucashuy commented Oct 4, 2023

I'm not Daniel :), but it should still work for non build in source flows.

The esbuild workflow defines a default building directory. When the workflow is initialized, it sets the build directory based on if build in source is enabled, and if the workflow supports it.

The unit test should cover this case (build in source enabled vs disabled)

@mndeveci
Copy link
Contributor

mndeveci commented Oct 4, 2023

I'm not Daniel :), but it should still work for non build in source flows.

The esbuild workflow defines a default building directory. When the workflow is initialized, it sets the build directory based on if build in source is enabled, and if the workflow supports it.

The unit test should cover this case (build in source enabled vs disabled)

I was summoning him for his feedback but I see that you already answered my question :)

@mildaniel
Copy link
Contributor

What Lucas said 😆 This change shouldn't impact the existing workflow at all

@lucashuy lucashuy added this pull request to the merge queue Oct 4, 2023
Merged via the queue into aws:develop with commit 90b6357 Oct 4, 2023
123 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants