Skip to content
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

Adding merge_commit_sha field to PullRequest model #260

Merged
merged 11 commits into from Jul 16, 2019

Conversation

jdesiloniz
Copy link

This PR adds the merge_commit_sha to the PullRequest model. According to the GitHub API docs:

The value of the merge_commit_sha attribute changes depending on the state of the pull request. Before merging a pull request, the merge_commit_sha attribute holds the SHA of the test merge commit. After merging a pull request, the merge_commit_sha attribute changes depending on how you merged the pull request:

If merged as a merge commit, merge_commit_sha represents the SHA of the merge commit.
If merged via a squash, merge_commit_sha represents the SHA of the squashed commit on the base branch.
If rebased, merge_commit_sha represents the commit that the base branch was updated to.

This field can be really useful for creating and updating statuses by targetting the test commit sha instead of the latest one in a PR.

@fedefernandez
Copy link
Contributor

@jdesiloniz I guess we need to disable that option for scala 2.11, something like:

scalacOptions ++= (scalaBinaryVersion.value match {
  case x if x.startsWith("2.12") => Seq("-Ywarn-macros:after")
  case _                         => Seq.empty
})

Copy link

@bilki bilki left a comment

Choose a reason for hiding this comment

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

This LGTM, aside from the compilation problems (that seem to be solved?)

@@ -124,6 +124,10 @@ object ProjectPlugin extends AutoPlugin {
scalaVersion := V.scala212,
crossScalaVersions := Seq(V.scala211, V.scala212),
scalacOptions ~= (_ filterNot Set("-Xlint").contains),
scalacOptions ++= ((scalaBinaryVersion.value, getEnvVar("SCALAENV")) match {
Copy link

Choose a reason for hiding this comment

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

Is this still necessary?

Copy link
Author

Choose a reason for hiding this comment

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

@bilki I've just launched a new build process in Travis without these, to verify if the stack size fix did the trick by itself... 🤔

Copy link
Author

Choose a reason for hiding this comment

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

@bilki yup, that did the trick! Thanks for the suggestion!!

version.sbt Outdated
version in ThisBuild := "0.20.2"
Copy link

Choose a reason for hiding this comment

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

Are we going to release now?

Copy link
Author

Choose a reason for hiding this comment

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

@bilki I'm holding back from this, so we can decide the release timeline appropiatedly in a separate PR.

@jdesiloniz jdesiloniz merged commit 0656a6a into master Jul 16, 2019
@jdesiloniz jdesiloniz deleted the feature/jsandino/add-merge-commit-sha-to-pulls branch July 16, 2019 16:58
Copy link
Contributor

@fedefernandez fedefernandez left a comment

Choose a reason for hiding this comment

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

retroactive approval, looks good to me

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.

None yet

3 participants