Issue 629: Refactor code duplication in AccountDiffPreference, PatchScriptSettings and PrettySettings
Status:  Released
Owner: ----
Closed:  Mar 2012
Project Member Reported by ziv...@gmail.com, Jul 19, 2010
Persisting of diff preferences was done in this issue:
https://code.google.com/p/gerrit/issues/detail?id=579

Since there was some code duplication in the
AccountDiffPreference vs (PatchScriptSettings + PrettySettings)
it was agreed to refactor it in a follow up issue.
Jul 19, 2010
Project Member #1 ziv...@gmail.com
It looks like this refactoring will require a change in project dependencies.
Here is an overview of the projects where the classes AccountDiffPreference,
PatchScriptSettings and PrettySettings belong and their dependencies:

Project         Class
gerrit-common   PatchScriptSettings
gerrit-prettify PrettySettings
gerrit-reviewdb AccountDiffPreference

Project         Dependends on
gerrit-common   gerrit-prettify
gerrit-common   gerrit-reviewdb
gerrit-reviewdb gwtorm

As far as I understand it, the AccountDiffPreference class must be there in the
reviewdb project in order to be persisted (because this is where we have
reference to gwtorm and can use @Column, @Relation and other annotations).
If we would like keep the AccountDiffPreference and remove the PrettySettings
and PatchScriptSettings then the problem would be that there is no dependency
from gerrit-prettify to gerrit-reviewdb and this dependency would have to
be introduced.

Any comments, opinions?

Status: Accepted
Jul 19, 2010
#2 sop@google.com
It should be OK to add gerrit-reviewdb as a dependency on
the gerrit-prettify component.  The only reason its not there
is because we didn't need anything.  By adding the new
dependency to the project we should be able to reduce some
code, and make the system a little easier to follow, so its
well worth the dependency change.
Jul 22, 2010
Project Member #3 ziv...@gmail.com
Fixed with commit: 8e33d76853200f922b170aed7412761cd5c16dbb
Status: Fixed
Mar 27, 2012
#4 sop@google.com
(No comment was entered for this change.)
Status: Released