bug#23945: 25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud

bug#23945: 25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud

[Teodor Zlatanov]

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

instead of gnus-cloud.el

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

The term "cloud computing" clouds users minds; it discourages them
from asking crucial questions, such as "Whose computer is that, in
which country, and what exactly are they asking me to entrust to it?"
It fuzzes all the different uses of the network together.  So we urge
people to reject the term "cloud."  See
http://gnu.org/philosophy/words-to-avoid.html.

Thus, I was perturbed to see that a file in Gnus grants legitimacy
to the term "cloud" by using it.  The name ought to be changed.

I was going to suggest another name based on what the file does.
But I can't figure out what it does.  The only explanation is one
line at the top of the file, and it doesn't say much.  That is
bad maintenance.

Would someone like to add comments sufficient to understand
what the code does?  And please show them to me.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: instead of gnus-cloud.el

[Lars Magne Ingebrigtsen]

Richard Stallman <rms@gnu.org> writes:

> Thus, I was perturbed to see that a file in Gnus grants legitimacy
> to the term "cloud" by using it.  The name ought to be changed.

The name is humorous -- a parody of the term "cloud computing".

> I was going to suggest another name based on what the file does.
> But I can't figure out what it does.  The only explanation is one
> line at the top of the file, and it doesn't say much.  That is
> bad maintenance.

It's a feature under development, and is not used by anybody, so
documentation of this non-existing feature would be premature.  Up until
very recently we weren't quite sure what it was going to look like, but
it's shaping up now.

It's a mechanism for storing data (in the first implementation, Gnus
data) on your local IMAP server, so that you can access your Emacs
environment from all the machines you use Emacs.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: instead of gnus-cloud.el

[Ted Zlatanov]

On Tue, 12 Jul 2016 01:10:55 -0400 Richard Stallman <rms@gnu.org> wrote: 

RS> The term "cloud computing" clouds users minds; it discourages them
RS> from asking crucial questions, such as "Whose computer is that, in
RS> which country, and what exactly are they asking me to entrust to it?"
RS> It fuzzes all the different uses of the network together.  So we urge
RS> people to reject the term "cloud."  See
RS> http://gnu.org/philosophy/words-to-avoid.html.

RS> Thus, I was perturbed to see that a file in Gnus grants legitimacy
RS> to the term "cloud" by using it.  The name ought to be changed.

RS> I was going to suggest another name based on what the file does.
RS> But I can't figure out what it does.  The only explanation is one
RS> line at the top of the file, and it doesn't say much.  That is
RS> bad maintenance.

Like Lars, I think this is funny because it's entirely unlike the
"cloud" in "cloud computing." It lets the user store their files, Gnus,
and other settings on an IMAP server they can control.

If "cloud" is really unacceptable, this replaces gnus-sync.el so maybe
it could take that name. But I would rather explain the joke in the
commentary.

Ted

Re: instead of gnus-cloud.el

[Lars Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> If "cloud" is really unacceptable, this replaces gnus-sync.el so maybe
> it could take that name. But I would rather explain the joke in the
> commentary.

It would be a nice place to put the required "the cloud is just someone
else's computer" saying into the Emacs manual.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: instead of gnus-cloud.el

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > It's a feature under development, and is not used by anybody, so
  > documentation of this non-existing feature would be premature.

Writing for the manual later is ok, but it should have comments in the
file now, don't you think?

  > The name is humorous -- a parody of the term "cloud computing".

What is the joke?

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: instead of gnus-cloud.el

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > If "cloud" is really unacceptable, this replaces gnus-sync.el so maybe
  > it could take that name. But I would rather explain the joke in the
  > commentary.

That joke might be funny, if it were presented clearly enough that
people could get it.

But please don't put the joke in the file name.  Most people who see
the file name won't know it is a joke.  With them, it will really encourage
the harmful "cloud" meme.

The name of something generally has more PR effect than the substance.
If the effect of the name is bad, it is impossible for the substance
to cancel that out.  So always use a name that doesn't have a bad PR
effect.

The joke can be presented in another way.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: instead of gnus-cloud.el

[Stefan Monnier]

> It's a mechanism for storing data (in the first implementation, Gnus
> data) on your local IMAP server, so that you can access your Emacs
> environment from all the machines you use Emacs.

BTW, the text says "imap" but the code (and the name) doesn't look
specific to imap (other than a call to nnimap-request-articles).


        Stefan

bug#23945: 25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud

[Ted Zlatanov]

On Mon, 11 Jul 2016 11:05:55 -0400 Teodor Zlatanov <tzz@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

Re: instead of gnus-cloud.el

[Ted Zlatanov]

On Tue, 12 Jul 2016 17:50:31 +0200 Lars Ingebrigtsen <larsi@gnus.org> wrote: 

On Wed, 13 Jul 2016 09:06:34 -0400 Richard Stallman <rms@gnu.org> wrote: 

>> If "cloud" is really unacceptable, this replaces gnus-sync.el so maybe
>> it could take that name. But I would rather explain the joke in the
>> commentary.

RS> That joke might be funny, if it were presented clearly enough that
RS> people could get it.

RS> But please don't put the joke in the file name.  Most people who see
RS> the file name won't know it is a joke.  With them, it will really encourage
RS> the harmful "cloud" meme.

OK, I'll let you and Lars and the maintainers figure it out. Let's do
the rename, if that's the decision, after the current gnus-cloud code is
reviewed and merged, or it will get confusing (because the feature
branch currently removes the old gnus-sync.el).

On Wed, 13 Jul 2016 09:04:48 -0400 Richard Stallman <rms@gnu.org> wrote: 

RS> Writing for the manual later is ok, but it should have comments in the
RS> file now, don't you think?

I already documented many of the gnus-cloud.el functions in my feature
branch. I'll add more docs to the code and expand the manual after it's
merged, since it's still in progress.

On Wed, 13 Jul 2016 09:52:56 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

SM> BTW, the text says "imap" but the code (and the name) doesn't look
SM> specific to imap (other than a call to nnimap-request-articles).

Yeah, indeed it may work with nnml right now, but we want to use IMAP
features in the future. For instance I think the native IMAP unique
article ID from https://tools.ietf.org/html/rfc3501#section-2.3.1 will
be better than the current `gnus-cloud-sequence' defvar. And IMAP flags
may be good for separating full vs. incremental updates, which currently
are encoded in the article subject.

Ted

bug#23945: 25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud

[Ted Zlatanov]

On Wed, 13 Jul 2016 10:20:12 -0400 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Mon, 11 Jul 2016 11:05:55 -0400 Teodor Zlatanov <tzz@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

bug#23945: 25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud

[npostavs]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Wed, 13 Jul 2016 10:20:12 -0400 Ted Zlatanov <tzz@lifelogs.com> wrote: 
>
> TZ> On Mon, 11 Jul 2016 11:05:55 -0400 Teodor Zlatanov <tzz@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.

bug#23945: 25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud

[Ted Zlatanov]

On Mon, 18 Jul 2016 19:18:09 -0400 npostavs@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

bug#23945: 25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud

[Ted Zlatanov]

On Tue, 19 Jul 2016 10:50:07 -0400 Ted Zlatanov <tzz@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

bug#23945: 25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud

[Noam Postavsky]

On Tue, Jul 19, 2016 at 4:15 PM, Ted Zlatanov <tzz@lifelogs.com> wrote:
> On Tue, 19 Jul 2016 10:50:07 -0400 Ted Zlatanov <tzz@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).

bug#23945: 25.1.50; Request for review: Gnus Cloud work in scratch/gnus-cloud

[Ted Zlatanov]

On Tue, 19 Jul 2016 21:36:12 -0400 Noam Postavsky <npostavs@users.sourceforge.net> wrote: 

NP> On Tue, Jul 19, 2016 at 4:15 PM, Ted Zlatanov <tzz@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