Skip to content

v1.19 - Moving embedded bit from offset to name in reflect and reflectlite #1248

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

v1.19 - Moving embedded bit from offset to name in reflect and reflectlite #1248

wants to merge 2 commits into from

Conversation

grantnelson-wf
Copy link
Collaborator

@grantnelson-wf grantnelson-wf commented Nov 8, 2023 •
edited
Loading

I updated the reflect and reflectlite package to set the embedded flag as part of the Go 1.19 upgrade.

Commit e1e66a0 "move embedded bit from offset to name" changes how reflect and reflectlite store the embedded flag. The creation of the name and the creation for structField is affected by this change for Go 1.19. Both are overwritten by GopherJS.

@nevkontakte
Copy link
Member

Thank you for the pull request! Sorry for a delayed review, this week has been busy at work...

I've created a go1.19 branch for you, so that we don't have to worry about breaking master while work is in progress. Could you please rebase you PR on top of it and use it as a target branch? That will ensure CI runs all tests against Go 1.19.

In terms of testing changes locally, I recommend installing Go 1.19.13 as a separate installation. Then, you can build and test standard library packages with GopherJS on Go 1.19.x

 $ go1.19.13 install . && gopherjs test -v testing

(Note if you have explicit GOROOT set in your shell, you'll need to point GOPHERJS_GOROOT at you go1.19.13 installation.

@nevkontakte nevkontakte self-requested a review November 12, 2023 16:43
@grantnelson-wf grantnelson-wf changed the base branch from master to go1.19 November 13, 2023 18:46
@grantnelson-wf grantnelson-wf changed the title Adding transition to v1.19 for reflect and reflectlite v1.19 - Moving embedded bit from offset to name in reflect and reflectlite Nov 13, 2023
nevkontakte
nevkontakte reviewed Nov 13, 2023
View reviewed changes
Copy link
Member

@nevkontakte nevkontakte left a comment

Choose a reason for hiding this comment

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

Looks good so far! I would suggest squashing the commits, one logical change per commit makes it easy to navigate the history. It also helps with keeping the history more linear 🙂

@grantnelson-wf
Adding v1.19 transition for reflect and reflectlite
64e0f75 
Reduced to v1.19 only

Update Go version to 1.19.13 to begin work on Go 1.19 support.
@grantnelson-wf
Copy link
Collaborator Author

Since this was branched off of master and I changed the base branch to go1.19, I'm having trouble getting the squash to work while not including the merge of go1.19 into this branch. I'm just going to close this one, and start a clean branch off of go1.19.

Clean PR: #1250

@grantnelson-wf grantnelson-wf deleted the moveEmbeddedBit branch November 14, 2023 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nevkontakte nevkontakte nevkontakte left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@grantnelson-wf @nevkontakte