[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#39912: [PATCH] Open describe-function links in view-mode (Bug#39912)
From: |
Nicholas Savage |
Subject: |
bug#39912: [PATCH] Open describe-function links in view-mode (Bug#39912) |
Date: |
Tue, 08 Sep 2020 20:14:15 -0400 |
User-agent: |
Cyrus-JMAP/3.3.0-259-g88fbbfa-fm-20200903.003-g88fbbfa3 |
Thanks for the comments. I've attached an updated patch, and have responded to
your points below.
On Tue, Sep 8, 2020, at 17:01, Stefan Kangas wrote:
> "Nicholas Savage" <nick@nicksavage.ca> writes:
>
> > I've written a patch for Bug #39912, as discussed in debbugs. I think I
> > have formatted this correctly and have followed CONTRIBUTE as best as I
> > could.
>
> Thanks! I have some comments.
>
> (Someone else will have to help you with getting the assignment process
> started.)
This is now underway!
>
> > I've also changed it so the actual source code link opens in view-mode
> > as well. I don't know if there are ramifications of that, so I'm happy
> > to change it if necessary. My thought process was that
> > describe-function is used as a reference to the code, not as a point
> > of entry for editing functions, so changing that is consistent with
> > changing the NEWS links, but I don't know what I don't know.
>
> Actually, I use it to edit the source code all the time. That's what
> that button is mostly used for, in my use. :-)
>
> So I think we'd better leave that part out.
>
Yeah, that makes sense now that I'm thinking about it. I thought it may be
beyond the pale, which is why I wanted to highlight it. I've removed it now in
the new attached patch.
> > * Open describe-function links in view-mode (Bug#39912) to make
> > behavior consistent depending on whether or not user has permission
> > to edit the files.
>
> The description is okay, but could be shorter (and should mention the
> link to NEWS button as discussed above). I'm also not sure if its
> necessary to discuss file permissions here, since those details will be
> in the bug report. Mentioning view-mode should be enough.
>
> Also, this doesn't seem to follow the ChangeLog format. If you use VC
> (or Magit) to edit the files, you can simply put point on the newly
> added line and type `C-x 4 a' to get it automatically generated. You
> could even do it directly in the ELisp file, if point is in the changed
> function. (Read CONTRIBUTE for more details.)
I didn't know about `C-x 4 a'. Is that better now? I have reworded the
description as well.
I appreciate all of your advice and help! This has been my first real attempt
at working in a development environment, as my day job is in a different
industry, so there's a bit of a learning curve but I'm eager to catch up and
contribute as I can.
0001-Open-describe-function-NEWS-links-in-view-mode-Bug-3.patch
Description: Text Data