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

Update devcontainer.json #514

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Update devcontainer.json #514

merged 3 commits into from
Aug 27, 2024

Conversation

ravicodelabs
Copy link
Contributor

@ravicodelabs ravicodelabs commented Aug 7, 2024

Updates the devcontainer.json file for both GitHub Codespaces as well as local VS Code docker setups (when devcontainer.json is being used). This shouldn't change much for folks using these tools already, but some improvements are made on the devcontainer.json.

@ravicodelabs ravicodelabs self-assigned this Aug 9, 2024
@ravicodelabs ravicodelabs marked this pull request as draft August 10, 2024 15:32
@ravicodelabs ravicodelabs marked this pull request as ready for review August 12, 2024 02:52
Copy link
Contributor

@gcardonag gcardonag left a comment

Choose a reason for hiding this comment

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

Approved ✅
Before merging, and for the benefit of folks who may review this PR in the future, what was the goal of these improvements?

The startup speed seems similar at a glance, but I recall talking to you about improvements for local dev where it may have not been working.

@ravicodelabs
Copy link
Contributor Author

Approved ✅

Before merging, and for the benefit of folks who may review this PR in the future, what was the goal of these improvements?

The startup speed seems similar at a glance, but I recall talking to you about improvements for local dev where it may have not been working.

Ah yes, the main goal here was to address an issue where the prior image would not work on an M processor based MacBook, as their was a conflict in the 'universal' image which did not allow the interoperability.

Regarding the fix: the configuration was updated to use a 'javascript-node' image rather than 'universal', which doesn't have the problematic dependency, and node is sufficient for our code base.

@gcardonag
Copy link
Contributor

Approved ✅

Before merging, and for the benefit of folks who may review this PR in the future, what was the goal of these improvements?

The startup speed seems similar at a glance, but I recall talking to you about improvements for local dev where it may have not been working.

Ah yes, the main goal here was to address an issue where the prior image would not work on an M processor based MacBook, as their was a conflict in the 'universal' image which did not allow the interoperability.

Regarding the fix: the configuration was updated to use a 'javascript-node' image rather than 'universal', which doesn't have the problematic dependency, and node is sufficient for our code base.

Thank you @ravicodelabs ! 😄

@gcardonag gcardonag merged commit 23abede into develop Aug 27, 2024
6 checks passed
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