GNU bug report logs -
#23945
25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud
Previous Next
Reported by: Teodor Zlatanov <tzz <at> lifelogs.com>
Date: Mon, 11 Jul 2016 15:07:02 UTC
Severity: wishlist
Found in version 25.1.50
Done: Ted Zlatanov <tzz <at> lifelogs.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 23945 in the body.
You can then email your comments to 23945 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23945; Package
emacs.
(Mon, 11 Jul 2016 15:07:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Teodor Zlatanov <tzz <at> lifelogs.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Mon, 11 Jul 2016 15:07:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
I've published the proposed changes to the Gnus Cloud code in the branch
`scratch/gnus-cloud' and would like to request a code review. It's a
single commit which I've tested over the last 2 weeks with the help of
other Gnus users.
I'll add documentation if the code is acceptable.
Thank you
Ted
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23945; Package
emacs.
(Wed, 13 Jul 2016 14:21:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 23945 <at> debbugs.gnu.org (full text, mbox):
On Mon, 11 Jul 2016 11:05:55 -0400 Teodor Zlatanov <tzz <at> lifelogs.com> wrote:
TZ> I've published the proposed changes to the Gnus Cloud code in the branch
TZ> `scratch/gnus-cloud' and would like to request a code review. It's a
TZ> single commit which I've tested over the last 2 weeks with the help of
TZ> other Gnus users.
TZ> I'll add documentation if the code is acceptable.
Lars? You look like a good reviewer.
There is one piece I remembered was missing: making the cloud host
persistent. After restarting Gnus, you have to set the gnus-cloud-method
with `gnus-server-toggle-cloud-method-server' again. How would you
suggest to save that, with a defcustom in gnus-cloud or in the
individual server's parameters? Right now it's just
(defvar gnus-cloud-method nil
"The IMAP select method used to store the cloud data.")
If I save it in the individual server's parameters there's a possibility
that two servers will have the cloud host flag accidentally.
Ted
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23945; Package
emacs.
(Mon, 18 Jul 2016 14:09:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 23945 <at> debbugs.gnu.org (full text, mbox):
On Wed, 13 Jul 2016 10:20:12 -0400 Ted Zlatanov <tzz <at> lifelogs.com> wrote:
TZ> On Mon, 11 Jul 2016 11:05:55 -0400 Teodor Zlatanov <tzz <at> lifelogs.com> wrote:
TZ> I've published the proposed changes to the Gnus Cloud code in the branch
TZ> `scratch/gnus-cloud' and would like to request a code review. It's a
TZ> single commit which I've tested over the last 2 weeks with the help of
TZ> other Gnus users.
TZ> I'll add documentation if the code is acceptable.
TZ> There is one piece I remembered was missing: making the cloud host
TZ> persistent.
I converted `gnus-cloud-method' to a defcustom and added the necessary
code to set it, resolving this question.
Since no one has been interested in reviewing this code, I will merge it
tomorrow.
Ted
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23945; Package
emacs.
(Mon, 18 Jul 2016 23:19:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 23945 <at> debbugs.gnu.org (full text, mbox):
Ted Zlatanov <tzz <at> lifelogs.com> writes:
> On Wed, 13 Jul 2016 10:20:12 -0400 Ted Zlatanov <tzz <at> lifelogs.com> wrote:
>
> TZ> On Mon, 11 Jul 2016 11:05:55 -0400 Teodor Zlatanov <tzz <at> lifelogs.com> wrote:
> TZ> I've published the proposed changes to the Gnus Cloud code in the branch
> TZ> `scratch/gnus-cloud' and would like to request a code review. It's a
> TZ> single commit which I've tested over the last 2 weeks with the help of
> TZ> other Gnus users.
>
> TZ> I'll add documentation if the code is acceptable.
>
> TZ> There is one piece I remembered was missing: making the cloud host
> TZ> persistent.
>
> I converted `gnus-cloud-method' to a defcustom and added the necessary
> code to set it, resolving this question.
>
> Since no one has been interested in reviewing this code, I will merge it
> tomorrow.
Perhaps this is partly because I'm not familiar with the code (or what
"Gnus Cloud" is), but it seems to me that you're missing a good summary
line explaining what any of these changes are for. The bug title and
commit message summary line mentions only "Gnus Cloud work". What
"work"? This feels actively reviewer-hostile.
The 2nd commit titled "Minor gnus-cloud UI improvements" is a bit better
(at least we can tell it's about UI), though adding 20 or so "Merge
branch 'master' of git.sv.gnu.org:/srv/git/emacs" in between doesn't
help much either.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23945; Package
emacs.
(Tue, 19 Jul 2016 14:51:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 23945 <at> debbugs.gnu.org (full text, mbox):
On Mon, 18 Jul 2016 19:18:09 -0400 npostavs <at> users.sourceforge.net wrote:
>> I converted `gnus-cloud-method' to a defcustom and added the necessary
>> code to set it, resolving this question.
>>
>> Since no one has been interested in reviewing this code, I will merge it
>> tomorrow.
n> Perhaps this is partly because I'm not familiar with the code (or what
n> "Gnus Cloud" is), but it seems to me that you're missing a good summary
n> line explaining what any of these changes are for. The bug title and
n> commit message summary line mentions only "Gnus Cloud work". What
n> "work"? This feels actively reviewer-hostile.
You're right. The history of this work goes back at least 4 years in the
Gnus mailing list so I was hoping Lars or someone else familiar with
that history would jump in. You can start with the most recent log of
work I did:
http://thread.gmane.org/gmane.emacs.gnus.general/85543/focus=87091
I will write docs for the feature, but simply wasn't sure about some key
aspects of it that depended on the code.
Please note I did add docstrings for most of the functions, especially
the interactive ones. But I agree that's no excuse for a lack of good
commit messages.
n> The 2nd commit titled "Minor gnus-cloud UI improvements" is a bit better
n> (at least we can tell it's about UI), though adding 20 or so "Merge
n> branch 'master' of git.sv.gnu.org:/srv/git/emacs" in between doesn't
n> help much either.
Well, that's an issue with the source code repo. I've mentioned it
before. Here's my rant:
I merge the master branch often because I have been using the gnus-cloud
branch and want to make sure it's not broken against master. I also test
and use many other new features. I really don't want to start working
outside the master branch.
But I can't rebase and push again, because the repo doesn't support
non-fast-forward pushes. I have to delete and recreate the branch
(notification emails x2, and a little bit of manual labor). But also
every time I do a push with merges, notification e-mails come out for my
changes plus everything merged. I can't win either way.
So the bottom line for me is, I'd really like notification e-mails not
to go out for "scratch/*" branches, or to be able to do a
non-fast-forward push to my scratch branch. Since neither is possible,
I'm doing the best I can for now. I am strongly considering using Github
next time (Git supports multiple remotes for a branch, so this is not
too hard) and saving myself all this trouble.
You can use `git log --no-merges' meanwhile to filter out those merge
commits. Sorry for the inconvenience.
Ted
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23945; Package
emacs.
(Tue, 19 Jul 2016 20:16:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 23945 <at> debbugs.gnu.org (full text, mbox):
On Tue, 19 Jul 2016 10:50:07 -0400 Ted Zlatanov <tzz <at> lifelogs.com> wrote:
TZ> You're right. The history of this work goes back at least 4 years in the
TZ> Gnus mailing list so I was hoping Lars or someone else familiar with
TZ> that history would jump in. You can start with the most recent log of
TZ> work I did:
TZ> http://thread.gmane.org/gmane.emacs.gnus.general/85543/focus=87091
TZ> I will write docs for the feature, but simply wasn't sure about some key
TZ> aspects of it that depended on the code.
I rebased and added docs for this feature. So now there's just three
commits in the scratch/gnus-cloud branch.
Let me know, based on the docs, what the commit message should say. I
can't think of a good summary, since the work is really widespread in
order to make this package useful.
Thanks
Ted
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23945; Package
emacs.
(Wed, 20 Jul 2016 01:37:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 23945 <at> debbugs.gnu.org (full text, mbox):
On Tue, Jul 19, 2016 at 4:15 PM, Ted Zlatanov <tzz <at> lifelogs.com> wrote:
> On Tue, 19 Jul 2016 10:50:07 -0400 Ted Zlatanov <tzz <at> lifelogs.com> wrote:
>
> TZ> You're right. The history of this work goes back at least 4 years in the
> TZ> Gnus mailing list so I was hoping Lars or someone else familiar with
> TZ> that history would jump in. You can start with the most recent log of
> TZ> work I did:
>
> TZ> http://thread.gmane.org/gmane.emacs.gnus.general/85543/focus=87091
>
> TZ> I will write docs for the feature, but simply wasn't sure about some key
> TZ> aspects of it that depended on the code.
>
> I rebased and added docs for this feature. So now there's just three
> commits in the scratch/gnus-cloud branch.
>
> Let me know, based on the docs, what the commit message should say. I
> can't think of a good summary, since the work is really widespread in
> order to make this package useful.
Maybe something like "Bring gnus-cloud.el into working order", I
gather from that thread that it was broken before, right? That's still
not very descriptive but it looks like there's just a lot of little
changes that aren't really tied together. Perhaps it would be better
broken up into more commits (though that's probably not worth the
trouble at this point).
Reply sent
to
Ted Zlatanov <tzz <at> lifelogs.com>:
You have taken responsibility.
(Wed, 20 Jul 2016 12:57:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Teodor Zlatanov <tzz <at> lifelogs.com>:
bug acknowledged by developer.
(Wed, 20 Jul 2016 12:57:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 23945-done <at> debbugs.gnu.org (full text, mbox):
On Tue, 19 Jul 2016 21:36:12 -0400 Noam Postavsky <npostavs <at> users.sourceforge.net> wrote:
NP> On Tue, Jul 19, 2016 at 4:15 PM, Ted Zlatanov <tzz <at> lifelogs.com> wrote:
>> Let me know, based on the docs, what the commit message should say. I
>> can't think of a good summary, since the work is really widespread in
>> order to make this package useful.
NP> Maybe something like "Bring gnus-cloud.el into working order", I
NP> gather from that thread that it was broken before, right? That's still
NP> not very descriptive but it looks like there's just a lot of little
NP> changes that aren't really tied together. Perhaps it would be better
NP> broken up into more commits (though that's probably not worth the
NP> trouble at this point).
All right... I squashed with the message you suggested. It's pushed so
I'll call this review done.
Ted
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Thu, 18 Aug 2016 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 9 years ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.