Re: Elisp branch ready for merge (??)

[Christopher Allan Webber]

Christopher Allan Webber writes:

> Well, I didn't think I'd have time to do this (and in a sense I didn't)
> but:
>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>
> I've rebased the whole branch against git master and added ChangeLog
> style entries.  "make check" is passing.  It seems to me that it's ready
> for merge.  I did the best I could on the ChangeLog additions, both with
> my limited ChangeLog experience and from my limited Guile internals
> experience.  So, corrections welcome, but otherwise...
>
> I think we really should not delay on this, and we should try to merge
> this as soon as possible.  This already has bitrotted before, and if we
> wait on it, it could bitrot again.  It would be great to get this pulled
> into Guile proper.
>
> Plus it would be a nice bullet point in the next release! :)
>
>  - Chris

I've been given commit access to Guile proper (horray!) and I pushed a
rebased version of this branch to origin at wip-elisp!  Have fun!

 - Chris

Re: Elisp branch ready for merge (??)

[Christopher Allan Webber]

Taylan Ulrich Bayırlı/Kammer writes:

> Christopher Allan Webber  writes:
>
>> Taylan Ulrich Bayırlı/Kammer writes:
>>
>>> Christopher Allan Webber  writes:
>>>
 Well, I didn't think I'd have time to do this (and in a sense I didn't)
 but:
   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip

 I've rebased the whole branch against git master and added ChangeLog
 style entries.  "make check" is passing.  It seems to me that it's ready
 for merge.  I did the best I could on the ChangeLog additions, both with
 my limited ChangeLog experience and from my limited Guile internals
 experience.  So, corrections welcome, but otherwise...
>>>
>>> Exciting!
>>>
>>> Small remark: the "title" line of the commit messages should be complete
>>> sentences.
>>
>> Yes, though I didn't write them.  I also don't know in each case what a
>> complete sentence would be.  I did my best job by filling in the
>> ChangeLog style part.  Often times my figuring out the ChangeLog stuff
>> was based on some sentence fragment.
>>
>> Changing the one part that is the original author's writing to something
>> different... I can do it by attempting to guess, but I'm worried about
>> removing that context.
>>
>> One thing I could do is leave in the description: "Original title: foo"
>>
>> What do you think of that?
>
> In some cases it seems the title is already a sentence, just lacking
> capitalization and punctuation.  (It seems Guile doesn't have a strict
> rule about the punctuation though.)
>
> In other cases it seems like titles that should be "Add foobar" are
> shortened to just "foobar", e.g.:
>
> - guile-backtrace function
> - eval-when
> - fset macro
> - defsubst
> - compiler macros
> - elisp @@ macro
>
> All in all it looks like most titles should be straightforward to fix.
> Fixing any would be better than fixing none IMO.

You are probably right.

> And I'd say mentioning the original title is unnecessary for the
> obvious ones, though we should get Robin's sanctioning for what we do.
> Is Robin perhaps available for some basic feedback?

I haven't heard from them, but if they chose to reach out it would be of
course most welcome!

 - Chris

Re: Elisp branch ready for merge (??)

[Taylan Ulrich Bayırlı/Kammer]

Christopher Allan Webber  writes:

> Taylan Ulrich Bayırlı/Kammer writes:
>
>> Christopher Allan Webber  writes:
>>
>>> Well, I didn't think I'd have time to do this (and in a sense I didn't)
>>> but:
>>>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>>>
>>> I've rebased the whole branch against git master and added ChangeLog
>>> style entries.  "make check" is passing.  It seems to me that it's ready
>>> for merge.  I did the best I could on the ChangeLog additions, both with
>>> my limited ChangeLog experience and from my limited Guile internals
>>> experience.  So, corrections welcome, but otherwise...
>>
>> Exciting!
>>
>> Small remark: the "title" line of the commit messages should be complete
>> sentences.
>
> Yes, though I didn't write them.  I also don't know in each case what a
> complete sentence would be.  I did my best job by filling in the
> ChangeLog style part.  Often times my figuring out the ChangeLog stuff
> was based on some sentence fragment.
>
> Changing the one part that is the original author's writing to something
> different... I can do it by attempting to guess, but I'm worried about
> removing that context.
>
> One thing I could do is leave in the description: "Original title: foo"
>
> What do you think of that?

In some cases it seems the title is already a sentence, just lacking
capitalization and punctuation.  (It seems Guile doesn't have a strict
rule about the punctuation though.)

In other cases it seems like titles that should be "Add foobar" are
shortened to just "foobar", e.g.:

- guile-backtrace function
- eval-when
- fset macro
- defsubst
- compiler macros
- elisp @@ macro

All in all it looks like most titles should be straightforward to fix.
Fixing any would be better than fixing none IMO.  And I'd say mentioning
the original title is unnecessary for the obvious ones, though we should
get Robin's sanctioning for what we do.  Is Robin perhaps available for
some basic feedback?

Taylan

Re: Elisp branch ready for merge (??)

[Mathieu Lirzin]

Christopher Allan Webber  writes:

> Mathieu Lirzin writes:
>
[...]
>> Small nitpick.  Could you remove the extra spaces at the start of
>> indented lines?  :)
>>
>> I know this practice is/was a common practice in Guile but even if it
>> looks prettier with indentation, this is not the proper change log
>> format described by GCS and recognized by 'change-log-mode' in Emacs.
>
> I could... but I'm hesitant to do so if it's not the standard ways to do
> things in Guile's repository.  I'll leave that question to whoever is
> interested in merging it.  If they'd like me to change it, I'll do it,
> and since they have commit access, I consider them the authority.

Sure, it will be an opportunity to discuss this major issue!  ;)

-- 
Mathieu Lirzin

Re: Elisp branch ready for merge (??)

[Christopher Allan Webber]

Taylan Ulrich Bayırlı/Kammer writes:

> Christopher Allan Webber  writes:
>
>> Well, I didn't think I'd have time to do this (and in a sense I didn't)
>> but:
>>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>>
>> I've rebased the whole branch against git master and added ChangeLog
>> style entries.  "make check" is passing.  It seems to me that it's ready
>> for merge.  I did the best I could on the ChangeLog additions, both with
>> my limited ChangeLog experience and from my limited Guile internals
>> experience.  So, corrections welcome, but otherwise...
>
> Exciting!
>
> Small remark: the "title" line of the commit messages should be complete
> sentences.

Yes, though I didn't write them.  I also don't know in each case what a
complete sentence would be.  I did my best job by filling in the
ChangeLog style part.  Often times my figuring out the ChangeLog stuff
was based on some sentence fragment.

Changing the one part that is the original author's writing to something
different... I can do it by attempting to guess, but I'm worried about
removing that context.

One thing I could do is leave in the description: "Original title: foo"

What do you think of that?

>> I think we really should not delay on this, and we should try to merge
>> this as soon as possible.  This already has bitrotted before, and if we
>> wait on it, it could bitrot again.  It would be great to get this pulled
>> into Guile proper.
>>
>> Plus it would be a nice bullet point in the next release! :)
>
> +1
>
> Would also be good that one doesn't need to keep two versions of Guile
> around when hacking on Guile-Emacs.

I agree! :)

Re: Elisp branch ready for merge (??)

[Christopher Allan Webber]

Mathieu Lirzin writes:

> Christopher Allan Webber  writes:
>
>> Well, I didn't think I'd have time to do this (and in a sense I didn't)
>> but:
>>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>>
>> I've rebased the whole branch against git master and added ChangeLog
>> style entries.  "make check" is passing.  It seems to me that it's ready
>> for merge.  I did the best I could on the ChangeLog additions, both with
>> my limited ChangeLog experience and from my limited Guile internals
>> experience.  So, corrections welcome, but otherwise...
>
> Nice!
>
> Small nitpick.  Could you remove the extra spaces at the start of
> indented lines?  :)
>
> I know this practice is/was a common practice in Guile but even if it
> looks prettier with indentation, this is not the proper change log
> format described by GCS and recognized by 'change-log-mode' in Emacs.

I could... but I'm hesitant to do so if it's not the standard ways to do
things in Guile's repository.  I'll leave that question to whoever is
interested in merging it.  If they'd like me to change it, I'll do it,
and since they have commit access, I consider them the authority.

Re: Elisp branch ready for merge (??)

[Mathieu Lirzin]

Christopher Allan Webber  writes:

> Well, I didn't think I'd have time to do this (and in a sense I didn't)
> but:
>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>
> I've rebased the whole branch against git master and added ChangeLog
> style entries.  "make check" is passing.  It seems to me that it's ready
> for merge.  I did the best I could on the ChangeLog additions, both with
> my limited ChangeLog experience and from my limited Guile internals
> experience.  So, corrections welcome, but otherwise...

Nice!

Small nitpick.  Could you remove the extra spaces at the start of
indented lines?  :)

I know this practice is/was a common practice in Guile but even if it
looks prettier with indentation, this is not the proper change log
format described by GCS and recognized by 'change-log-mode' in Emacs.

-- 

Mathieu Lirzin

Re: Elisp branch ready for merge (??)

[Taylan Ulrich Bayırlı/Kammer]

Christopher Allan Webber  writes:

> Well, I didn't think I'd have time to do this (and in a sense I didn't)
> but:
>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>
> I've rebased the whole branch against git master and added ChangeLog
> style entries.  "make check" is passing.  It seems to me that it's ready
> for merge.  I did the best I could on the ChangeLog additions, both with
> my limited ChangeLog experience and from my limited Guile internals
> experience.  So, corrections welcome, but otherwise...

Exciting!

Small remark: the "title" line of the commit messages should be complete
sentences.

> I think we really should not delay on this, and we should try to merge
> this as soon as possible.  This already has bitrotted before, and if we
> wait on it, it could bitrot again.  It would be great to get this pulled
> into Guile proper.
>
> Plus it would be a nice bullet point in the next release! :)

+1

Would also be good that one doesn't need to keep two versions of Guile
around when hacking on Guile-Emacs.

>  - Chris

Taylan

Elisp branch ready for merge (??)

[Christopher Allan Webber]

Well, I didn't think I'd have time to do this (and in a sense I didn't)
but:
  https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip

I've rebased the whole branch against git master and added ChangeLog
style entries.  "make check" is passing.  It seems to me that it's ready
for merge.  I did the best I could on the ChangeLog additions, both with
my limited ChangeLog experience and from my limited Guile internals
experience.  So, corrections welcome, but otherwise...

I think we really should not delay on this, and we should try to merge
this as soon as possible.  This already has bitrotted before, and if we
wait on it, it could bitrot again.  It would be great to get this pulled
into Guile proper.

Plus it would be a nice bullet point in the next release! :)

 - Chris