Re: [Gimp-developer] refactor palette loading code

[Jehan Pagès <jehan marmottard gmail com>]

Hi,

On Wed, Sep 11, 2013 at 1:10 PM, Michael Henning
<drawoc darkrefraction com> wrote:
> I made a few minor nitpicks on your commit on github. If you would fix
> those points, I'll commit your code to master.

ok cool, someone reviewed the patch before me. :-D

> As a side note for the future, the fastest way to get a patch reviewed
> is normally if you post it to a pastebin and bother people on irc.

For my own, I would prefer a git format-patch like this, but on a
feature request/bug report on bugzilla. That's easy to patch a branch
and to remove after. And also we keep track of any discussion or
updated patch about a feature/fix. For instance go find this email
thread in one year in the mailing history.

>   -- drawoc
>
> P.S. I don't see the patch on that last email. Are you sure you attached it?

I see it but I was also a direct recipient. I guess that the list
cleans emails out from any attached file.
Could we have conditional filters? Like any text file with a ".patch"
or ".diff" extension should not be filtered out.

Jehan

> On Mon, Sep 9, 2013 at 3:27 AM, Warren Turkal <wt penguintechs org> wrote:
> > Attached is the patch.
> >
> > wt
> >
> >
> > On Sun, Sep 8, 2013 at 12:13 PM, Warren Turkal <wt penguintechs org> wrote:
> >
> > > Hey,
> > >
> > > I will get the format-patch as soon as I can. However, if you want to do
> > > it the git way and get it sooner, you can always add my remote to your git
> > > with the following command:
> > > git remote add wt git github com:wt/gimp.git
> > >
> > > Note that "wt" can be whatever alias you want, but my commands below
> > > assume you use "wt" as the remote alias.
> > >
> > > That url for the repo is available on the main page for the repo on that
> > > website after a single click. To find it from the commit page that I sent
> > > earlier (this link here<https://github.com/wt/gimp/commit/d1e8c4c8543b18c6f5d95f6ab6b3bbbf8f80778b>),
> > > look near the top left. You can see it if you look toward the top left.
> > > You'll see something like "PUBLIC wt/gimp". If you click on the "gimp"
> > > part, it will take you the repo main page (here<https://github.com/wt/gimp>).
> > > Look on the right side of the page. You can get https, ssh, and a few other
> > > types of urls for the repo.
> > >
> > > Once you have my remote, you can fetch my repo objects with this commands:
> > > git fetch wt
> > >
> > > Then, you can do all the git operations you want. For example, here's how
> > > to get a log:
> > > git log wt/refactor_palette_loader
> > >
> > > If you want to merge in my commit, do the following while on your local
> > > working (maybe "master") branch:
> > > git merge wt/refactor_palette_loader
> > >
> > > And if you find that you don't want my remote in your repo anymore, you
> > > can remove it. Make sure you don't have any branches tracking mine. One way
> > > deal with branches tracking mine is to just delete them. Then execute this
> > > command:
> > > git remote remove wt
> > >
> > > After doing that, you can also do the following if you to get rid of
> > > commits that were only in my repo:
> > > git prune
> > >
> > > wt
> > >
> > >
> > > On Sun, Sep 8, 2013 at 5:33 AM, Jehan Pagès <jehan marmottard gmail com>wrote:
> > >
> > > > Hi,
> > > >
> > > > I searched a little in this web UI, but couldn't find: is there a way
> > > > to generate a proper patch from this? Otherwise, could you generate
> > > > one with "git format-patch origin/master", run on your locale branch?
> > > > Thanks.
> > > >
> > > > Jehan
> > > >
> > > >
> > > > On Mon, Sep 9, 2013 at 12:20 AM, Warren Turkal <wt penguintechs org>
> > > > wrote:
> > > > > BTW, I tested this with gpl, act, aco, pal, and css palette files. I
> > > > > couldn't find RIFF file. If anyone has one, would you mind emailing it
> > > > to
> > > > > me so that I can try loading it?
> > > > >
> > > > > Thanks,
> > > > > wt
> > > > >
> > > > >
> > > > > On Sun, Sep 8, 2013 at 5:06 AM, Warren Turkal <wt penguintechs org>
> > > > wrote:
> > > > > > Hi again,
> > > > > >
> > > > > > Here's a link<
> > > > https://github.com/wt/gimp/commit/d1e8c4c8543b18c6f5d95f6ab6b3bbbf8f80778b>to
> > > > a commit containing a refactor of the palette loading code. I have moved
> > > > > > the file open/close logic to common code. This change actually removes
> > > > more
> > > > > > code than it adds. Here's a interesting diffstat (without whitespace
> > > > only
> > > > > > changes):
> > > > > >
> > > > > > $ git diff --stat --color -w origin/master
> > > > > >  app/core/gimp.c               |   4 +-
> > > > > >  app/core/gimppalette-import.c |  37 +++++++++--
> > > > > >  app/core/gimppalette-load.c   | 148
> > > > > > ++++++++++++-----------------------------
> > > > > >  app/core/gimppalette-load.h   |  12 +++-
> > > > > >  4 files changed, 85 insertions(+), 116 deletions(-)
> > > > > >
> > > > > > Any chance this could be pulled into the master? Do y'all have any
> > > > other
> > > > > > thoughts on this?
> > > > > >
> > > > > > Thanks,
> > > > > > wt
> > > > > _______________________________________________
> > > > > gimp-developer-list mailing list
> > > > > List address:    gimp-developer-list gnome org
> > > > > List membership:
> > > > https://mail.gnome.org/mailman/listinfo/gimp-developer-list
> > _______________________________________________
> > gimp-developer-list mailing list
> > List address:    gimp-developer-list gnome org
> > List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list