Skip to content

Conversation

@bahner
Copy link

@bahner bahner commented Jan 28, 2024

As writing files is not really applicable in WebAssembly create a noop filewriter to make compilation possible.

@bahner bahner requested a review from a team as a code owner January 28, 2024 09:48
@welcome
Copy link

welcome bot commented Jan 28, 2024

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

  • The context for this PR is clear, with relevant discussion, decisions
    and stakeholders linked/mentioned.

  • Your contribution itself is clear (code comments, self-review for the
    rest) and in its best form. Follow the code contribution
    guidelines

    if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
Next steps:

  • A maintainer will triage and assign priority to this PR, commenting on
    any missing things and potentially assigning a reviewer for high
    priority items.

  • The PR gets reviews, discussed and approvals as needed.

  • The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution.
We are very grateful for your contribution!

@bahner
Copy link
Author

bahner commented Jan 28, 2024

Incidentally, I believe this closes issue #186

@hacdias hacdias requested a review from Jorropo January 29, 2024 09:23
@bahner
Copy link
Author

bahner commented Feb 4, 2024

@Jorropo when you take a look at this, then feel free to suggest actual writing to in-memory files. I just don't see any reason, but if there is then just holler.

@Jorropo
Copy link
Contributor

Jorropo commented Feb 5, 2024

I would like to support WASM too but I want to you know that you havn't added anything to the test suite.
If we don't test WASM in CI I could merge it but with expectations that it's not maintained, experimental and could break at any moment.

Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

I don't think a new filewriter_wasm file is correct.
The STD emulates a unix filesystem on top of JS:
https://cs.opensource.google/go/go/+/master:src/os/file_unix.go;l=5?q=openFileNolog&ss=go%2Fgo

@@ -1,4 +1,6 @@
//go:build darwin || linux || netbsd || openbsd || freebsd || dragonfly || js || wasip1
//go:build darwin || linux || netbsd || openbsd || freebsd || dragonfly || (js && !wasm) || wasip1
// +build darwin linux netbsd openbsd freebsd dragonfly js,!wasm wasip1
Copy link
Contributor

@Jorropo Jorropo Feb 5, 2024

Choose a reason for hiding this comment

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

Btw, it's been since 1.18 that +build isn't used anymore.
I think you should upgrade your version of go. 😉 🙂

@bahner
Copy link
Author

bahner commented Feb 13, 2024

Hepp! Good feedback. A little busy here, but I'll follow up.

@lidel lidel added need/author-input Needs input from the original author kind/test Testing work labels Apr 16, 2024
@lidel lidel mentioned this pull request Jun 25, 2024
@gammazero gammazero removed the need/author-input Needs input from the original author label Aug 20, 2024
@gammazero gammazero marked this pull request as draft August 20, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/test Testing work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants