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

Komax refactoring factor methods #849

Closed
wants to merge 5 commits into from

Conversation

Projects
None yet
3 participants
@komax
Copy link
Member

komax commented Jul 2, 2013

Refactoring on:

  • descriptive identifiers
  • factorization of long conditions into separate variable
  • private helper methods for code duplicates
  • small amount @override

@komax komax referenced this pull request Jul 2, 2013

Closed

Komax codereview #847

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Jul 11, 2013

I think verifybytecode looks like a good change to me but you might want to isolate current JIT code changes to it's own PR and perhaps only make the verifyByteCode() method as the sole PR. @headius can decide whether he likes the change since he primarily works in that package space.

The Ruby try { inputstream.close() } bulletproofing can also be it's own PR. We generally don't bulletproof code against things which should not happen (although we should have had an assert there) but your pattern is quite common and someone will likely eventually submit that change again.

You can make a massive @override PR and I will apply that so long as it has no other changes. We have not been up to date with that in a long time. Make sure you @Oveeride with respect to Java 6 and not Java 7 though.

@enebo enebo closed this Jul 11, 2013

@headius

This comment has been minimized.

Copy link
Member

headius commented Jul 11, 2013

Yeah re Java 6 versus Java 7; I believe Java 6 will error if you @override an interface method...so make sure any change you make works on 6 and 7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.