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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Summer has come, Spring is gone #798

Merged
merged 1 commit into from Apr 1, 2019

Conversation

Projects
None yet
5 participants
@tinchou
Copy link
Contributor

commented Mar 29, 2019

This is the culmination of a long-term investment in improving our dependency injection code. We cleaned circular dependencies, removed all @Autowired private fields, and many other things that enabled us to do this mostly-automatic refactor.

Spring modules were removed, shaving off 4MB of our distributable JAR.

This is also a step forward in the direction of migrating to a newer Java release (Spring 4.2.9 was old, and we would've had to update at least to version 5.1).

_ 馃 build using fed:remove_spring_

image

@tinchou tinchou requested review from diega and lsebrie Mar 29, 2019

@tinchou tinchou force-pushed the remove_spring branch from 80b45c3 to 4007c45 Mar 29, 2019

@tinchou tinchou force-pushed the remove_spring branch from 4007c45 to 03cbc0f Mar 29, 2019

@rskops

This comment has been minimized.

Copy link

commented Mar 29, 2019

SonarQube analysis reported 52 issues

  • MAJOR 19 major
  • MINOR 26 minor
  • INFO 7 info

Top 10 issues

  1. MAJOR BlockstoreBlockPlayer.java#L45: Replace this use of System.out or System.err by a logger. rule
  2. MAJOR BlockstoreBlockPlayer.java#L45: Format string should use %n rather than \n in co.rsk.BlockstoreBlockPlayer.connectBlocks() rule
  3. MAJOR BlockstoreBlockPlayer.java#L46: Remove this call to "exit" or ensure it is really required. rule
  4. MAJOR BlockstoreBlockPlayer.java#L46: co.rsk.BlockstoreBlockPlayer.connectBlocks() invokes System.exit(...), which shuts down the entire virtual machine rule
  5. MAJOR BlockstoreBlockPlayer.java#L51: Replace this use of System.out or System.err by a logger. rule
  6. MAJOR BlockstoreBlockPlayer.java#L51: Method co.rsk.BlockstoreBlockPlayer.connectBlocks() appears to call the same method on the same object redundantly rule
  7. MAJOR BlockstoreBlockPlayer.java#L51: Format string should use %n rather than \n in co.rsk.BlockstoreBlockPlayer.connectBlocks() rule
  8. MAJOR BlockstoreBlockPlayer.java#L55: Replace this use of System.out or System.err by a logger. rule
  9. MAJOR BlockstoreBlockPlayer.java#L69: Replace this use of System.out or System.err by a logger. rule
  10. MAJOR RskContext.java#: This file has 1,104 lines, which is greater than 1,000 authorized. Split it into smaller files. rule
@diega

diega approved these changes Mar 30, 2019

Copy link
Contributor

left a comment

This looks extremely awesome.
We should take a few minutes to thank the Spring Framework for all its good practices that, well applied, made us to get rid of it so cleanly.

I would evaluate what about instantiating the whole graph in the constructor of the RskContext, that would mimic closely how the Spring ApplicationContext works, right now it's like we were using the @Lazy annotation all the time.

The king is dead, long live the king!

@lsebrie lsebrie merged commit 2994564 into master Apr 1, 2019

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
default Build finished.
Details
sonarqube SonarQube reported 52 issues, no criticals or blockers

@lsebrie lsebrie deleted the remove_spring branch Apr 1, 2019

@aeidelman aeidelman added this to the Orchid v0.6.2 milestone Apr 19, 2019

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