Skip to content

Conversation

@Hweinstock
Copy link
Contributor

Problem

Changing the permissions of a file is currently not supported by the file system wrapper.

Solution

Implement a stub chmod that does nothing on web, and uses fs to modify permissions in non-web environments.


License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Hweinstock Hweinstock changed the title filesystem: add stub for chmod to filesystem wrapper feat(filesystem): add stub for chmod to filesystem wrapper Sep 23, 2024
@github-actions
Copy link

This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions.

@Hweinstock
Copy link
Contributor Author

/retryBuilds

@Hweinstock Hweinstock marked this pull request as ready for review September 23, 2024 17:55
@Hweinstock Hweinstock requested a review from a team as a code owner September 23, 2024 17:55

it('sets permission of the file to read/write owner', async function () {
if (!globals.isWeb && os.platform() !== 'win32') {
const result = await stat(keyPair.getPrivateKeyPath())
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I guess we will need fs.stat(), later...

Copy link
Contributor

@justinmk3 justinmk3 left a comment

Choose a reason for hiding this comment

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

Thanks! Are there any existing usages we can replace too?

@Hweinstock Hweinstock requested a review from a team as a code owner September 24, 2024 23:06
Copy link
Contributor

@justinmk3 justinmk3 left a comment

Choose a reason for hiding this comment

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

LGTM let's merge it!

@Hweinstock Hweinstock merged commit 37d97c4 into master Oct 1, 2024
14 of 18 checks passed
@Hweinstock Hweinstock deleted the hkobew/filesystem/chmod branch October 1, 2024 13:27
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