Extended refactoring support

[michael holzer]

Hi everybody,

for quite some time now I've been working on new refactorings in the
Scala refactoring library and their integration in the Scala IDE. Now
the time has come and the pull request is ready [1].

So, what is new?

- Method signature refactorings:
- Change parameter order (within parameter lists)
- Split parameter lists
- Merge parameter lists

- Extract trait
Extracts class members (val/var/def) to a new trait

- Move constructor to companion object
Redirects calls constructor calls to a newly generated apply-method of
the companion object.
If necessary, the companion object is generated as well.

- Source generators:
- Generate hashCode and equals
- Introduce ProductN trait
Generates methods to implement ProductN for selected class parameters.
This includes generation of hashCode and equals.

Hopefully these will be useful additions, any comment are of course welcome!

michael

[1] https://github.com/scala-ide/scala-ide/pull/111

[Richard O. Legendi]

Wow, great features! Why don't pull requests have a Like button? :-)

Best,
Richard

[Som Snytt]

Maybe someday:

On Thu, May 24, 2012 at 9:57 AM, Richard O. Legendi <richard...@gmail.com> wrote:

Wow, great features! Why don't pull requests have a Like button? :-)

From: Tekkub (GitHub Staff)
Subject: Why don't pull requests have a Like button?

Thanks for the feedback. I've added your suggestion to the Feature Request List™ for the team to see.

[Ben Hutchison]

This is a great effort. However from pull request, I see there 1436
lines of Scala added yet no tests.

(I don't work for or represent Typesafe in any way, but) if the
codebase maintainers accept this pull request, how do they

(a) know if the submitted code actually works? Manually tests it all I guess?
(b) support the added features and code in future? If there are bugs
in the new refactorings, will it be Typesafe who get blamed?

So there is an ongoing cost in adding this to the codebase, that needs
to be managed.

-Ben

[Mirko Stocker]

That's a good point; but we don't have any UI tests in the Scala IDE,
and the bulk of this contribution is UI related. The actual
refactoring implementations are all part of the scala-refactoring
library, and there we have hundreds of tests (last time I checked,
test coverage was 97% or so).

Cheers,

Mirko

--
Mirko Stocker | m...@misto.ch
Work: http://ifs.hsr.ch | http://infoq.com
Personal: http://misto.ch | http://twitter.com/m_st

[iulian dragos]

Hi Ben,

You are raising a very good question: how should the maintainers
accept code contributions in an open-source project? We try to keep a
reasonable high-bar for contributions, and sometimes the code-review
process delays the merge significantly. Bu the way, it's great to have
others in the community looking at the code, so thank you for that!
You can also just add comments on the pull-request, and start a
discussion on a relevant piece of code. The more feedback, the better
we become.

Regarding blame and praise, I think they (should) go to the authors.
We're trying to promote the IDE project as an open-source, independent
project, even though the biggest contributor right now *is* Typesafe.
Whenever we accept patches, we implicitly accept supporting that part
of the code, but people tend to stick around, and we're happy to have
long-time contributors like Mirko and Matt maintaining their code.

In this specific case, Mirko answered already: there are no UI tests
in the IDE (that's another discussion), and almost all the code in
this pull request is a GUI over the scala-refactoring library. As with
any external library, we implicitly 'trust' it, and if we find bugs,
we'll report them in the library's issue tracker. And indeed, when
reviewing a contribution I usually build that branch and give it a try
for a day or two. That tends to find not only bugs, but also usability
issues (or performance regressions). Not ideal, but has some
advantages.

cheers,
iulian

On Wed, May 30, 2012 at 2:44 AM, Ben Hutchison <brhut...@gmail.com> wrote:

--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais