Code behind MyPreferencesScreen

[richar...@googlemail.com]

Hi,

I just went through adding a new preference and all the wiring behind it to MyPreferencesScreen. (You can't see the change yet, because my contributor agreement, a corporate one, is not yet through.) This appeared more difficult to me than it should be, since lots of small additions in lots of classes have to be made. And I spent a long time figuring out why my code didn't work, i.e. which addition I had overlooked to make.

So I wondered if it has to be so difficult or if some refactoring would be a possible and useful effort. Should I apply myself to it or not?

Best,

Richard

[Edwin Kempin]

Adding a new user preference is really that painful, lots of different places in the code need to be touched :(
To find all the places it's best to look at an old change that added such a preference, e.g. [1].

I guess everyone would be glad if this would be simpler, but nobody took the time to simplify it.

[1] https://gerrit-review.googlesource.com/49061

 

Best,

Richard

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[richar...@googlemail.com]

Am Mittwoch, 5. November 2014 18:05:14 UTC+1 schrieb Edwin Kempin:

Adding a new user preference is really that painful, lots of different places in the code need to be touched :(
To find all the places it's best to look at an old change that added such a preference, e.g. [1].

I guess everyone would be glad if this would be simpler, but nobody took the time to simplify it.

[1] https://gerrit-review.googlesource.com/49061

Yeah, thanks. I actually did it like that, but I still overlooked something at first.

Since I don't have many burning issues I want to tackle, I might think of a way to simplify the whole thing. I'm not very experienced with largish codebases, though, so I don't know if I will be successful. Any hint is welcome!

[Edwin Kempin]

I guess this is not really an easy task.

We do have a similar problem with project configuration parameters, where also a lot of code places need to be
touched if a new parameter is introduced.

Nowadays plugins can extend the project configuration and here we have a really nice solution [1].

All a plugin needs to do is to define the parameter in a single place:

  bind(ProjectConfigEntry.class)
        .annotatedWith(Exports.named("enabled"))
        .toInstance(new ProjectConfigEntry("Enable Greeting", true));

All the rest is automatic. We have a generic way to transport the parameters (type, name, value, etc) via REST
to the client and on client-side we have generic UI code in ProjectInfoScreen to render these parameters.

Something similar could be done for the Gerrit core project parameters and also for user preferences.

If to tackle this for user preferences we should probably at the same time also change how the user preferences

are persisted. At the moment they are stored in the database, but we already have a AllUsers repository with a
branch per user where some first user preferences are stored. So this would be a good chance to move

the old user preferences over to this new persistence. They may make thinks also easier since you can just
store the user preferences in a git config file format, while in the database we currently need a schema migration

whenever a new user preference is introduced. Moving the user preferences into Git is anyway needed for the

NoteDB story, where all data should be stored in Git rather than in the database.

Well, but I guess all this is really a lot of work and probably the reason why nobody attempted this yet.

[1] https://gerrit-review.googlesource.com/Documentation/dev-plugins.html#simple-project-specific-configuration

[richar...@googlemail.com]

Am Freitag, 7. November 2014 09:58:25 UTC+1 schrieb Edwin Kempin:
[...]

Well, but I guess all this is really a lot of work and probably the reason why nobody attempted this yet.

Looks like it. Anyway, I already have some ideas for little cleanups, which might be a good start.

[David Ostrovsky]

Am Freitag, 7. November 2014 09:58:25 UTC+1 schrieb Edwin Kempin:

2014-11-05 21:35 GMT+01:00 <richar...@googlemail.com>:

Am Mittwoch, 5. November 2014 18:05:14 UTC+1 schrieb Edwin Kempin:

Adding a new user preference is really that painful, lots of different places in the code need to be touched :(
To find all the places it's best to look at an old change that added such a preference, e.g. [1].

I guess everyone would be glad if this would be simpler, but nobody took the time to simplify it.

[1] https://gerrit-review.googlesource.com/49061

Yeah, thanks. I actually did it like that, but I still overlooked something at first.

Since I don't have many burning issues I want to tackle, I might think of a way to simplify the whole thing. I'm not very experienced with largish codebases, though, so I don't know if I will be successful. Any hint is welcome!

I guess this is not really an easy task.

We do have a similar problem with project configuration parameters, where also a lot of code places need to be
touched if a new parameter is introduced.

Nowadays plugins can extend the project configuration and here we have a really nice solution [1].

All a plugin needs to do is to define the parameter in a single place:

  bind(ProjectConfigEntry.class)
        .annotatedWith(Exports.named("enabled"))
        .toInstance(new ProjectConfigEntry("Enable Greeting", true));

All the rest is automatic. We have a generic way to transport the parameters (type, name, value, etc) via REST
to the client and on client-side we have generic UI code in ProjectInfoScreen to render these parameters.

Something similar could be done for the Gerrit core project parameters and also for user preferences.

If to tackle this for user preferences we should probably at the same time also change how the user preferences

are persisted. At the moment they are stored in the database, but we already have a AllUsers repository with a
branch per user where some first user preferences are stored. So this would be a good chance to move

the old user preferences over to this new persistence.

I am facing the same challenge, to introduce a bunch of new Codemirror preferences needed for inline edit series [1].

Something we already have for Side By Side2 screen, but for edit mode.  I don't even think about extending the

database for new 5-10 preferences. The way to go is to store them in AllUsers repo. This rises the question though

what to do first: either introduce new CM3 preferences for inline edit series and store them in AllUsers repository, or

migrate existing user preferences to AllUsers repository first?

[1] https://gerrit-review.googlesource.com/61243

[Shawn Pearce]

On Sun, Nov 9, 2014 at 11:16 PM, David Ostrovsky <david.o...@gmail.com> wrote:

I am facing the same challenge, to introduce a bunch of new Codemirror preferences needed for inline edit series [1].

Something we already have for Side By Side2 screen, but for edit mode.  I don't even think about extending the

database for new 5-10 preferences. The way to go is to store them in AllUsers repo. This rises the question though

what to do first: either introduce new CM3 preferences for inline edit series and store them in AllUsers repository, or

migrate existing user preferences to AllUsers repository first?

[1] https://gerrit-review.googlesource.com/61243

Introduce the new preferences in AllUsers. This continues to set the pattern of how we would migrate the existing prefs.

Migrate the existing prefs later, in another commit, after we are happy with how the new prefs work.

[David Ostrovsky]

Done. Adding new user preference option in post database era decreases the signal-to-noise ratio of

resulting code [1].  Due to the nature of Gerrit as GWT based application, number of places must be

adjusted, though: two containers are maintained (client and server), including synchronization between

them (client caches the preferences representation to save round trips) and UI specific rendering code,

including event dispatching.

[1] https://gerrit-review.googlesource.com/61605