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

Improve devcontainer file structure #2854

Merged

Conversation

networkfusion
Copy link
Member

@networkfusion networkfusion commented Dec 27, 2023

Description

Motivation and Context

Although there are some futher changes I would like to make (in another PR), this moves the devcontainers to seperate directories which makes it easier for users to choose and develop against their respective platforms in a devcontainer.

It fixes the various issues raised against the last PR.

How Has This Been Tested?

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dev Containers (changes related with Dev Containers, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

@networkfusion networkfusion marked this pull request as ready for review December 27, 2023 16:00
@networkfusion networkfusion changed the title Devcontainer structure Improve devcontainer file structure Dec 27, 2023
@networkfusion
Copy link
Member Author

@alberk8 Given I know that you use devcontainers, if you can also test this and add feedback, it would be welcome.

@networkfusion networkfusion mentioned this pull request Dec 27, 2023
13 tasks
@alberk8
Copy link
Contributor

alberk8 commented Dec 28, 2023

@networkfusion, It works but rather annoying is that you need to select the Dev Container every time you reopen the container. Is it possible to have something like selecting the target where it will resists across Container restart?.

Copy link
Member

@Ellerbach Ellerbach left a comment

Choose a reason for hiding this comment

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

hat will definitely be much cleaner! And I think the action should be adjusted, see my comment

.github/workflows/devcontainer-esp32.yml Show resolved Hide resolved
.github/workflows/devcontainer-chibios.yaml Show resolved Hide resolved
@networkfusion networkfusion force-pushed the devcontainer-structure-2 branch 2 times, most recently from acbff0e to fea81cc Compare January 8, 2024 20:59
@networkfusion
Copy link
Member Author

@Ellerbach any response to the comments before merge?
@alberk8 I get it, but, the prompt to reopen the container at least makes sure that you are dealing with the correct container and has the benefit of making sure it is the latest version (whilst also more intuative for new users).

@Ellerbach
Copy link
Member

any response to the comments before merge?

go ahead, nothing we can't correct later anyway ;-)

@networkfusion networkfusion merged commit 641cd9e into nanoframework:main Jan 17, 2024
14 checks passed
@networkfusion networkfusion deleted the devcontainer-structure-2 branch January 17, 2024 14:21
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