Skip to content

feat: cli arg for file permission of socket - #4923

Merged
jsjoeio merged 1 commit into
coder:mainfrom
TechCiel:main
Mar 3, 2022
Merged

feat: cli arg for file permission of socket#4923
jsjoeio merged 1 commit into
coder:mainfrom
TechCiel:main

Conversation

@TechCiel

@TechCielTechCiel commented Feb 28, 2022

Copy link
Copy Markdown
Contributor

Fixes#1466

Add a cli option --socket-permission, and set file mode accordingly when listening a socket.

@TechCiel
TechCiel requested a review from a teamFebruary 28, 2022 14:07
@jsjoeiojsjoeio self-assigned this Feb 28, 2022
jsjoeio
jsjoeio approved these changes Feb 28, 2022
View reviewed changes

@jsjoeiojsjoeio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very clean PR 👏 Thank you for adding tests as well!

I'm less familiar with socket-permissions so I'll defer review to @code-asher

@jsjoeiojsjoeio added the feature New user visible feature label Mar 1, 2022
@codecov

codecovBot commented Mar 1, 2022•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #4923 (c1abea9) into main (78658f1) will increase coverage by 0.00%.
The diff coverage is 66.66%.

Impacted file tree graph

@@ Coverage Diff @@## main #4923 +/- ##
=======================================
Coverage 70.29% 70.30% =======================================
Files 29 29 Lines 1670 1677 +7 Branches 369 371 +2 =======================================
+ Hits 1174 1179 +5 - Misses 422 424 +2 
Partials 74 74
Impacted FilesCoverage Δ
src/node/app.ts94.82% <62.50%> (-3.25%)⬇️
src/node/cli.ts84.35% <100.00%> (+0.05%)⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78658f1...c1abea9. Read the comment docs.

@jsjoeiojsjoeio assigned code-asher and unassigned jsjoeioMar 1, 2022
code-asher
code-asher requested changes Mar 2, 2022
View reviewed changes

@code-ashercode-asher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the contribution! This has been a long-standing one.

Comment threadsrc/node/app.ts Outdated
Comment threadsrc/node/app.ts Outdated
@TechCiel
TechCiel requested a review from code-asherMarch 3, 2022 09:28
@jsjoeiojsjoeio added this to the 4.0.3 milestone Mar 3, 2022
@jsjoeio
jsjoeio merged commit c4d8758 into coder:mainMar 3, 2022
TinLe pushed a commit to TinLe/code-server that referenced this pull request Apr 23, 2022
@TechCiel@TinLe
feat: cli arg for file permission of socket (coder#4923)
4bfa9f8
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@code-ashercode-ashercode-asher approved these changes
+1 more reviewer
@jsjoeiojsjoeiojsjoeio approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@code-ashercode-asher

Labels

featureNew user visible feature

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

Add option to set unix socket permissions

3 participants

@TechCiel@jsjoeio@code-asher