Hi Denis and all,
just a few remark (as a non-Replicant developer) a.k.a my 2-cents:
The issue is that with merge commits it then becomes very hard and time
consuming to understand what really constitutes Replicant changes, so I
think that rebases are better.
I would argue, that rebasing looses quite important information, i.e. you
loose the state on which the changes had worked *and* had been tested. In
other words it will get harder to reconstruct the version that had worked
based on history of current branch. That has also implications if you try
to bisect. Sure, there are tags containing that etc., but it is just out
of current history. IMHO that's an anti-pattern (if that history could
matter in the future).
The issue is that with merge commits it then becomes very hard and time
consuming to understand what really constitutes Replicant changes, so I
think that rebases are better.
Shouldn't a plain git 'git log upstream/master..' do the job?
This shows all commits of current HEAD not present on 'upstream/master',
which IMHO is exactly what you want.
- I could point to a branch on a git repository but I'm unsure if it's
very convenient to review as it would increase the burden on the
people reviewing the patches, as they would need to go fetch that and
look at it instead of already having the patch to review right in the
mail.
Personally I prefer reviews with SCM support, that is I can use all my
beloved and well known tools for that. Obviously, for small changes (or if
it happens that you /just know/ all the context) it's easier to just review
the patch included in some mail. (It guess it works for the Linux kernel
developers since they are able cache a lot of context ;D)
Theoretically a patch workflow lowers the entry barrier, but I am not sure
if that is really true, since it's quite complicated to do it really right.
The advantage of a patch based workflow is, that you don't need some account
to contribute a patch (except access to the mailing list, of course) and that
everything is implicitly documented in "plain" email conversations.
Back to topic: I see the advantage of patch based workflow (no overhead to
set up your environment), but I would argue that a solution which provides that
view based on underlying SCM, would be the best solution.
In short: Is there any way to show that diffs in the web frontend of Git? It
seems 'cgit' is not capable of that. Perhaps such support can be added.
Regards,
doak
On 16.02.20 16:58, Denis 'GNUtoo' Carikli wrote:
On Sat, 15 Feb 2020 23:38:34 +0000
Fil Lupin <[email protected]> wrote:
Hi,
Hi,
don't know if needed but Ok for me.
Unfortunately I already pushed the last part of the patches that
weren't reviewed, and here we're the upstream so we can't easily
rewrite the git history.
The current procedure that is in use for Replicant 6.0 is to send
patches, and if there is no review for one week, the people that are
able to push patches can push them. If someone gets 1 positive review
before that, the people that can push are allowed to push.
Though it's not optimal:
- Here I sent a huge number of patches, that take time to review. Maybe
I should give more time in that case or just plainly ask if people
need more time to review them.
- Sometimes I've no review at all.
But it's still better than some other procedures:
- Several bugs were caught and fixed thanks to that procedure, and that
can even work with 0 reviews.
For instance I sent for review a patch that adds a script that wraps
Replicant 6.0 build, and I then changed my mind on some details of
the implementation.
Several patches were also improved thanks to the reviews.
- I tried waiting for reviews before pushing, and that leads to loosing
patches and/or becomes way to messy as some patches are never
reviewed and forgotten about. And since such patches are useful, it's
better to push them at some point than forgetting about them.
Rebasing Replicant 6:
---------------------
We have no review procedure for rebasing repositories like the
Replicant 6 repositories. In the past, merges commits were used to merge
Replicant changes with latest LineageOS or CyanogenMod changes.
The issue is that with merge commits it then becomes very hard and time
consuming to understand what really constitutes Replicant changes, so I
think that rebases are better.
So I've been rebasing some of the Replicant 6 repositories. The wiki
has a summary of that here:
https://redmine.replicant.us/projects/replicant/wiki/Replican6Changes
I think it would be a good idea to require reviews for that too. For
instance I probably broke booting of the Galaxy Nexus and Galaxy Tab
2 because of a rebase.
But the issue is that I'm unsure of what would be the most
convenient way of doing it:
- I could send the patches to be applied again on top of a known
revision. For instance lineageos/cm-13. I'm unsure how practical it
is to review as some of the information is not there (like it's
harder to do a diff with that).
- I could point to a branch on a git repository but I'm unsure if it's
very convenient to review as it would increase the burden on the
people reviewing the patches, as they would need to go fetch that and
look at it instead of already having the patch to review right in the
mail.
- I could do both: send patches, and also have them on a branch
somewhere. The time spent to do both is near 0 for me.
Also do people wanting to review that kind of work have some idea on
which information to include, like a diff between the old and the new
rebase, or ways that would make it easier to review?
For other repositories:
-----------------------
- Replicant 9 specific repositories have currently no review in place
as it's still under heavy development and can be rebased at any time.
Though some of the work is still reviewed as people working on it
often look at each other commits afterward, and some of the work is
going in libsamsung-ipc, Linux, and will also go in libsamsung-ril,
so that is being reviewed.
- Obviously, user repositories obviously require no reviews.
Denis.
_______________________________________________
Replicant mailing list
[email protected]
https://lists.osuosl.org/mailman/listinfo/replicant
_______________________________________________
Replicant mailing list
[email protected]
https://lists.osuosl.org/mailman/listinfo/replicant
