search

portable-scala / portable-scala-reflect

Platform independent reflection for Scala
BSD 3-Clause "New" or "Revised" License
42 stars 14 forks source link

JVM: Don't use deprecated Class#newInstance() #40

Closed sideeffffect closed 3 years ago

sideeffffect commented 3 years ago

From https://github.com/portable-scala/portable-scala-reflect/pull/39#discussion_r604651184

sideeffffect commented 3 years ago

@gzm0 those diffs are there so that it merges well with the original PR. It seemed easier this way. Or do you think it would be better to remove them anyway?

gzm0 commented 3 years ago

Or do you think it would be better to remove them anyway?

Yes I think removing them would be better. We should trade the other PR if there are any conflicts.

The reason for this is that we want a single PR to change one thing only (mich easier to understand the history that way).

sideeffffect commented 3 years ago

Sure thing! Done :heavy_check_mark:

gzm0 commented 3 years ago

LGTM. We can merge this after you squash all the commits. (unless @sjrd has further comments).

sideeffffect commented 3 years ago

@gzm0 I would be more than happy to do the squashing myself, but if you don't mind me asking, why don't you just do the Squash-merge of this PR yourself? GitHub makes it a trivial one-click-button operation :smile_cat:

gzm0 commented 3 years ago

Good question :) A bunch of reasons: On one hand habit from pre-GitHub support times. However, more importantly, Scala.js (and due to its history, this repo) uses --no-ff merges. So every merge creates an individual merge commit, even if the branch could be fast forwarded. Or in other words: The master branch only contains merge commits and special (release) commits directly. GitHub does not support this flavor of squash and merge.

Why do we do this: It allows us to have PRs that have multiple meaningful commits (which we use quite extensively in Scala.js core, for example: https://github.com/scala-js/scala-js/pull/4450).

sideeffffect commented 3 years ago

IMHO this PR is a good example where squash merge from GitHub UI would make things easier for everybody, but never mind, it's squashed now :wink: