Multiple accounts / Settings per project #382
Conversation
|
Wow, this is a huge change! Thanks for your work. I did not have the time to look into the code yet, but I've some questions:
If you do not mind, you could try to reduce the size of this change a little bit by removing all whitespace / indention only changes. |
Almost. However, I didn't touch the ui. I thought the change was big enough to warrant a subsequent PR to add the UI hint.
For the existing users, it will be transparent. I integrated a versioning system easing the settings migration process. The same system can be reused for future settings management modifications.
I tested the authentication, the code reviews grid listing and the settings migration. |
reda-alaoui
force-pushed
the
issue-159_per-project-settings
branch
from
ab58c8a to
02ac250
Compare
|
Whitespaces/indentation changes were removed |
|
Did a short test and works fine so far - great work! Just wondering if we probably should make all settings project specific... the header in the settings page shows "For current project" anyway (independent of your change) and some settings like "Push to Gerrit by default" makes only sense for projects which have Gerrit enabled. What do you think? |
|
I tried to do that at first. But I came to the conclusion that settings like the ones adding grid columns should be global. Most of the time, you want to toggle the grid columns for all projects.
IMO, « pushToGerritByDefault » is the only one left that should be moved. I want to do this in another pull request because the difficulty with this one is how we pull the settings from the main classloader using javassist.
Réda Housni Alaoui
… Le 28 avr. 2020 à 22:30, Urs Wolfer ***@***.***> a écrit :
Did a short test and works fine so far - great work!
Just wondering if we probably should make all settings project specific... the header in the settings page shows "For current project" anyway (independent of your change) and some settings like "Push to Gerrit by default" makes only sense for projects which have Gerrit enabled. What do you think?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Maybe I haven't been clear enough in this part. |
|
We could also consider that all settings should be project level because IntelliJ encourages it. From this point of view, we can consider that the poor ergonomics is IntelliJ's fault, not ours. If people were to complain about that, we could redirect them to this kind of feature request or comment. |
I do not have a strong preference here. Both approaches have their own downsides. I think we can go with the approach which keeps the code simpler. It's up to you. :) Will do more testing ASAP. |
|
In this case, I prefer to keep it simple for this PR ;) |
|
@reda-alaoui: Sorry for my late feedback. I've just tried to test it with a recent IntelliJ version against some production tests, but failed to do so. I've manged to merge your PR into Have you developed this change only against intellij14 branch or also against a recent version of it? You did everything as documented (develop PRs against intellij14), but in this case it might make sense to develop it only against the most recent version because it is such a heavy change and also IntelliJ related settings infrastructure changed after IntelliJ 14. Would it be possible for you to rebase this PR on |
|
@uwolfer ok I will try that asap |
|
@uwolfer I locally rebased on intellij2016.2 . I have no error while saving the settings. |
|
I'm getting errors after playing around in the plugin settings (but it could be possible that I've failed resolving some merge conflicts):
|
|
@reda-alaoui: If you do not find time to look into my exception, you could also start a new PR which is based on the |
|
@uwolfer Ok I will. Sorry I didn't have much time lately. |
reda-alaoui
force-pushed
the
issue-159_per-project-settings
branch
from
02ac250 to
0ea8199
Compare
|
@uwolfer I rebased the PR on intellij16.2 |
|
Hey, any progress on this? This would be super useful to have, I regularly interact with different Gerrit servers. Is there a donation pool or bounty platform to contribute to? Glad to contribute $100 to this effort. |
|
@leoluk: Unfortunately there is no progress from my side. I'm lacking a bit of time, but it would be really great if we could get that landed. The biggest help in this would be if somebody could test the whole change implemented by @reda-alaoui, also regarding backwards compatibility. |
|
Happy to help with that. What combinations need testing for backwards compatibility? |
|
@leoluk: Some constellations I'd like to have tested:
You can find a guide how to setup a development environment for this plugin here. You support is highly appreciated! |
|
I think this PR is officialy dead :) |
Fix #159