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

infra: setup dev-container #1652

Merged
merged 9 commits into from
Jan 17, 2023
Merged

infra: setup dev-container #1652

merged 9 commits into from
Jan 17, 2023

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Dec 11, 2022

Review Usage:

  • Install dev container extension in vscode (Docker, ...)
  • Right click on the PR -> clone in dev container
  • run pnpm run test, pnpm run docs:dev ... as you would do locally

Later Usage:

  • Install dev container extension in vscode (Docker, ...)
  • Clone Repo in Dev Container
  • run pnpm run test, pnpm run docs:dev ... as you would do locally

This is an optional feature, which may (or may not) help external contributors.

@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision c: infra Changes to our infrastructure or project setup labels Dec 11, 2022
@ST-DDT ST-DDT self-assigned this Dec 11, 2022
@codecov
Copy link

codecov bot commented Dec 11, 2022

Codecov Report

Merging #1652 (3442239) into next (d7c1718) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 3442239 differs from pull request most recent head 4d7ebd5. Consider uploading reports for the commit 4d7ebd5 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #1652   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files        2337     2336    -1     
  Lines      241178   241123   -55     
  Branches     1098     1102    +4     
=======================================
- Hits       240294   240241   -53     
+ Misses        863      861    -2     
  Partials       21       21           
Impacted Files Coverage Δ
src/definitions/company.ts 0.00% <0.00%> (ø)
src/modules/string/index.ts 100.00% <0.00%> (ø)
src/modules/company/index.ts 100.00% <0.00%> (ø)
src/utils/merge-locales.ts

@Shinigami92 Shinigami92 added the s: waiting for user interest Waiting for more users interested in this feature label Dec 11, 2022
@ST-DDT ST-DDT marked this pull request as ready for review December 19, 2022 00:41
@ST-DDT ST-DDT requested a review from a team as a code owner December 19, 2022 00:41
@ST-DDT ST-DDT requested a review from a team December 19, 2022 00:41
@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Dec 22, 2022

Review Usage:

  • Install dev container extension in vscode (Docker, ...)
  • Right click on the PR -> clone in dev container
  • run pnpm run test, pnpm run docs:dev ... as you would do locally

I'm sorry, I can get it to work. I don't have any experience with dev containers. Could you update the description to give a more detailed "guide" on how to review, or link to some resources on that?

I just checked this PR branch out and "Reopen in in container" locally.

Copy link
Member

@xDivisionByZerox xDivisionByZerox left a comment

Choose a reason for hiding this comment

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

After allowing the "unsafe" git repo (prompt from VS Code) I could do the following things:

  • ✅update code and commit it
  • ✅install the dependencies (happens automatically)
  • ✅build the project
  • ✅build the docs
  • ✅serve the docs
  • ✅run linter
  • ✅run tests

Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

I will approve so it's not blocking, but feel free to rethink about vscode recommendation
it could get annoying to recommend extensions that are not really needed at the end of the day

.vscode/extensions.json Show resolved Hide resolved
@Shinigami92
Copy link
Member

And updates for this PR?
It has two approvals for several days now
As I won't use this anyways it is not such important for me what is configured in it, so go on and merge as you like

@ST-DDT ST-DDT enabled auto-merge (squash) January 17, 2023 21:01
@ST-DDT ST-DDT added s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision s: waiting for user interest Waiting for more users interested in this feature labels Jan 17, 2023
@ST-DDT ST-DDT merged commit cc0b034 into next Jan 17, 2023
@ST-DDT ST-DDT deleted the feat/dev-container branch January 17, 2023 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants