Skip to content

Crash when calling Compiler.newRun() without a docCtx #13013

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.

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

Merged
merged 1 commit into from
Jul 5, 2021

Conversation

@som-snytt
Copy link
Contributor

You'll want to squash and force push one clean commit. But the change looks idiomatic.

It would be nice if the docCtx.get in TreePickler were idiomatic so grep doesn't notice it; that follows an assert. There is a getOrElse throw in QuotesImpl.

@smarter
Copy link
Member

smarter commented Jul 5, 2021

It looks like you haven't signed the CLA yet, would you mind doing so at https://www.lightbend.com/contribute/cla/scala ?

@GavinRay97
Copy link
Contributor Author

GavinRay97 commented Jul 5, 2021

Apologies for the delay, have signed the CLA.

Somewhat embarrassingly, though I use git every day for years, I'm not well-versed beyond basic checkout/commit/pull/branch/push.

To squash and force-push as a single commit, would be something like this?

$ git rebase -i HEAD~2 # Interactive rebase last 2 commits on local
# Select "Squash" from prompt
$ git commit
$ git push -f

Or

$ git reset --soft HEAD~2 && git commit
$ git push -f

@smarter
Copy link
Member

smarter commented Jul 5, 2021

There's multiple ways to do it, if you squash using git rebase -i, it will prompt you for the commit message of the squashed commit and generate it itself, so you don't need to git commit afterwards, but you'll indeed need to do git push -f.

@GavinRay97
Copy link
Contributor Author

Okay I think I managed to get it sorted out. Let me know if it's still not right and we can give this a go again 😅

@smarter
Copy link
Member

smarter commented Jul 5, 2021

It looks good but to have the issue be automatically closed when this PR is merged, you need to include "Fixes #12988" somewhere in the body of the commit message (also this is nitpicking but ideally the first line of the commit message would be a bit more descriptive of what is being fixed e.g., "Fix crash when calling Compiler.newRun() without a docCtx")

@smarter
Copy link
Member

smarter commented Jul 5, 2021

By the way, you can edit just the commit message by doing git commit --amend (and then git push -f again)

@GavinRay97
Copy link
Contributor Author

By the way, you can edit just the commit message by doing git commit --amend (and then git push -f again)

Thank you for this, I didn't know this and this definitely helps a lot.
Gimme two seconds, commit incoming =)

@smarter smarter changed the title #12998 ctx.docCtx.get -> ctx.docCtx.foreach Crash when calling Compiler.newRun() without a docCtx Jul 5, 2021
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@smarter smarter enabled auto-merge July 5, 2021 17:35
@smarter smarter merged commit f9c9421 into scala:master Jul 5, 2021
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.

3 participants