gopherjs / gopherjs/gopherjs

Sequencing PRs to be merged

Open
#793 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
13.2k
Forks
573
PR merge metrics
No merged PRs in 30d

Description

Hi @shurcooL @hajimehoshi - just wanted to create an issue to discuss the sequencing of the various open PRs we currently have on the slate:

  • #669: compiler: fix handling of struct, array and interface values (bug affecting the runtime-correctness of GopherJS programs)
  • #787: Embed core GopherJS packages into build system; enable vendoring of GopherJS
  • prelude changes:
    • #784: compiler: automate regeneration of prelude - MERGED
    • #791: compiler/prelude: move prelude to separate .js files
    • #794 automates the formatting of the separate .js files
  • #790: js: auto-generate getters and setters for calls to MakeWrapper

#669 has been discussed with @neelance before and just needs a final look before it can be merged (@shurcooL is aware). As this affects runtime-correctness it's pretty important. Originally reported in #661 and most recently, independently, in #783.

There is no dependency between #787 and any of the other listed PRs so this can be reviewed and merged in parallel to anything else we choose to do.

There is also no dependency between #790 and any of the other listed PRs, so like #787 this can go in at any time, but it would probably make sense to go in after #791 (and #784) given that it again modifies the prelude.

Prelude changes

We need to make a decision on whether we merge #784, #791 and #794 or just the latter. Ultimately I think we want to get all three merged, but they are separated out per @hajimehoshi to be clear what each does; they can therefore be merged in sequence if you prefer.

  • #784 - removes the manual step of minifying the prelude by introducing genmin.go. Unminified prelude is still driven from prelude.go, which represents the concatenated result of other .go parts of the prelude. Minified result is put in prelude_min.go - MERGED
  • #791 - moves the source prelude from .go files into .js files. genmin.go becomes genprelude.go and prelude.go becomes a generated result, along with prelude_min.go, of genprelude.go (which does the job of concatenating the .js files and minifying them, outputting to the appropriate file)
  • #794 (forked from #791 - diff) - automates the formatting of the .js prelude files (much like gofmt automates the formatting of .go files) via formatjsprelude.go

Would welcome your thoughts.

Thanks

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review the listed pull requests, especially #669, #787, #790, #791, and #794, along with the prelude.go, prelude_min.go, genprelude.go, and formatjsprelude.go entries mentioned in the issue. Determine the dependency and merge order, with completion defined as an agreed sequencing decision for the open work.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, javascript
Domain
build-system, compilers
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
15/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.