[vlc-devel] Essai envoi

Alexandre Janniaux ajanni at videolabs.io
Wed Oct 16 19:42:28 CEST 2019


Hi,

Yes, this is documented, but yes, this is an incomplete,
confusing not well-maintained documentation. It will
probably move to a different platform to ease maintenance
later.

Here are the documentation links:
+ https://wiki.videolan.org/Git/
+ https://wiki.videolan.org/Sending_Patches_VLC/

What you probably want to do is the following:
+ squash the commit when it is relevant, so that each commit
  is an atomic non-breaking change to the codebase.
+ git fetch origin && git rebase origin/master, given that
  origin is the VLC's main repository.
+ git format-patch --cover-letter --reroll-count=2 \
      --output-directory somefolder origin/master
  (I've let the verbose option so you can check what they
   are doing)
+ edit the cover letter and check the different patches in
  the somefolder directory.
+ git send-email --no-chain-reply-to [potentially
      --in-reply-to=<some Message-id>] \
      --to vlc-devel at videolan.org

When you'll get some ease at using send-mail, you can
shortcut everything (especially for little patch) using:

git send-email --to vlc-devel at videolan.org
    [-<number of commit you want to send> OR
     origin/master OR
     fromcommit..tocommit]
    [--reroll-count=2 ]
    --compose
    --no-chain-reply-to
    [--in-reply-to=<message-id> ]

Note that you can send email to anyone, not just the mailing
list, so you can actually test with your own inbox to check
how it is working. It is also a very convenient way to send
patches/some code to someone or receive some, especially if
the receiver has the mail environment to handle this
efficiently. For example with neomutt, I can just use `t` to
select the patches and then write ;|git am<enter> to apply
all selected patches to my current worktree. One could also
just bind a key for this, and I guess thunderbird can allow
the same sort of thing but never tried.

I hope everything is clear for you and that I didn't write
mistakes,

Regards,
--
Alexandre Janniaux
Videolabs

On Wed, Oct 16, 2019 at 11:53:17AM +0200, Jérôme Froissart wrote:
> Thanks for your reply.
> Just to be sure that I will not jeopardize (again) the mailing list, could
> you confirm I understood correctly? I should now :
> - squash the main commit and the change I have made after you reviewed it
> - send a fresh (that does not reply to anything) patchset to the mailing
> list
> - append a "(v2)" to the title of the cover letter of this fresh patchset
>
> Is that correct?
> Thanks for the help (maybe this workflow could be described on your wiki,
> for people like me who are contributing for the first time via a mailing
> list? Or maybe it is written already, but I failed to find it?)
>
> Jérôme
>
>
> On Tue, 15 Oct 2019, 14:15 Alexandre Janniaux, <ajanni at videolabs.io> wrote:
>
> > Hi,
> >
> > This looks correct and a good idea, but you will need to
> > send clean patches.
> >
> > > I am new to using mailing lists, so I hope I replied correctly and did
> > not mess with the --in-reply-to option.
> > > Also, I am sending a single "code review" commit, is that all right for
> > you? Or do you rather prefer one commit that squashes this correction with
> > the previous proposed patch?
> >
> > We usually prefer proper commit and use git range-diff with
> > the previous version to compare, so the patch can be merged
> > without resend.
> >
> > Also, you have to put the GPLv2 license for Qt files, as
> > the module is itself in GPLv2. You can grab it from any
> > file from the Qt module.
> >
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Mon, Oct 14, 2019 at 10:59:06AM +0200, Jérôme Froissart wrote:
> > > ---
> > > You are right, I forgot to call config_StringEscape.
> > > Here is a correction patch. Note that it escapes quotes and slashes, it
> > does not percent-encode special chars. I guess it is the correct behaviour
> > (since it was used in the implementation I removed), but please tell me if
> > some percent-escaping should take place as well.
> > >
> > > I am new to using mailing lists, so I hope I replied correctly and did
> > not mess with the --in-reply-to option.
> > > Also, I am sending a single "code review" commit, is that all right for
> > you? Or do you rather prefer one commit that squashes this correction with
> > the previous proposed patch?
> > >
> > > Thanks
> > >
> > >
> > >  modules/gui/qt/util/soutmrl.cpp | 7 ++++++-
> > >  modules/gui/qt/util/soutmrl.hpp | 2 +-
> > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/modules/gui/qt/util/soutmrl.cpp
> > b/modules/gui/qt/util/soutmrl.cpp
> > > index 90615c6c86..6dc316ec42 100644
> > > --- a/modules/gui/qt/util/soutmrl.cpp
> > > +++ b/modules/gui/qt/util/soutmrl.cpp
> > > @@ -14,7 +14,12 @@ QString MrlModule::to_string() const
> > >          s += it->first;
> > >          if( it->second.to_string().compare("") != 0 )
> > >          {
> > > -            s += "=" + it->second.to_string();
> > > +            char *psz = config_StringEscape(
> > qtu(it->second.to_string()) );
> > > +            if( psz )
> > > +            {
> > > +                s += "=" + qfu( psz );
> > > +                free( psz );
> > > +            }
> > >          }
> > >          ++it;
> > >          if( it != options.end() )
> > > diff --git a/modules/gui/qt/util/soutmrl.hpp
> > b/modules/gui/qt/util/soutmrl.hpp
> > > index 2e92e9064e..b18016fef3 100644
> > > --- a/modules/gui/qt/util/soutmrl.hpp
> > > +++ b/modules/gui/qt/util/soutmrl.hpp
> > > @@ -90,7 +90,7 @@ private:
> > >  ///     - any number of key(=value) pairs
> > >  ///       values can be nested modules
> > >  ///
> > > -/// Example of MRL:
> > HEADERmodule1{a,b=val}:module2:module3{opt,arg=1,stuff=nestedModule{subkey=subvalue}}
> > > +/// Example of MRL:
> > HEADERmodule1{a,b=val}:module2:module3{opt,arg=\"value with automatically
> > escaped quotes\",stuff=nestedModule{subkey=subvalue}}
> > >  class SoutMrl
> > >  {
> > >  public:
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel



More information about the vlc-devel mailing list