Re: [Gimp-developer] refactor palette loading code

[Michael Henning <drawoc darkrefraction com>]

I made a few minor nitpicks on your commit on github. If you would fix
those points, I'll commit your code to master.

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.

  -- drawoc

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

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