Renaming of ICLA PDF files - why?

[sebb <se...@gmail.com>]

The icla code attempts [1] to rename PDF files to icla.pdf and
icla.pdf.asc if there is a signature.

However the code has been broken for a while, because the fileext is
nil unless the signature is empty [2]. Thus the condition becomes:

if @signature.to_s.empty? or (not @signature.empty? && fileext != '.pdf')

i.e. the condition is always true.

AFAICT this has been the case since May 2017 [3]

It's easy enough to fix (ensure fileext is always created).

However I wonder if the rename is needed, and if so, why not rename
other files that have detached signatures? Removing the rename would
simplify the code, and make it easier to use the shared asvn_update
task code.

Note that most applications seem to use icla.pdf anyway.

Sebb

[1] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L68

[2] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L16

[3] https://github.com/apache/whimsy/commit/786413d74a2515f91916225d929705b7b5c08811

Re: Renaming of ICLA PDF files - why?

[sebb <se...@gmail.com>]

On Tue, 21 Jul 2020 at 23:43, Sam Ruby <ru...@intertwingly.net> wrote:
>
> On Tue, Jul 21, 2020 at 5:46 PM sebb <se...@gmail.com> wrote:
> >
> > On Tue, 21 Jul 2020 at 21:31, Sam Ruby <ru...@intertwingly.net> wrote:
> > >
> > > The case that needs to be handled is if I were to send in an updated
> > > ICLA by the name of 'sam-ruby.pdf'.  At that point, there would be a
> > > sam-ruby.pdf in the documents/iclas directory, and one in the email,
> > > and both would need to be placed into a directory.
> >
> > Indeed, but that's a separate issue.
> > The code does check if there is a matching ICLA and will put out a
> > warning if so.
> > [The user must then either change the target name or use ICLA2 if the
> > duplicate is intentional.]
> >
> > What I am referring to is the following:
> >
> > Assume there is an email with attachments named:
> > abcd.pdf
> > abcd.pdf.asc
> >
> > The code I am referring to will attempt to create the following files:
> > iclas/subdir/icla.pdf
> > iclas/subdir/icla.pdf.asc
>
> My recollection is that the original intent was that the next icla
> pdf/asc pair should be named icla2.pdf and icla2.pdf.asc.
>
> And the reason for this naming convention was to both prevent the
> naming conflict and to make it easier to determine the order in which
> the documents were received as this would make it easier to find the
> most recent document.
>
> > That does not work currently.
>
> The code could be fixed to do what I stated above.  Or it could be
> replaced with a different approach.
>
> > Even if it did, it would not fix the name clash issue.
> > Also the code does not rename other file types such as .txt.
>
> Possibly a second bug.
>
> > So why the rename?
>
> To prevent name clashes, and make it easier to identify the most
> recent document or document pair.

Using a fixed name makes name clashes more likely.
However it would make it easier to determine the sequential name
suffix to be used, provided that all file pairs used the same stem.

Now, in the case of documents stored under a directory, I think the
name stem is arbitrary, so why not use a value that is naturally
sequential?
That would avoid any need to look for existing files when filing another.

For example, one could use the sequential sequence: yyyymmdd[hhmm]

It probably is not necessary to use hours and minutes but it might
make testing easier.

It would make the name a bit longer, but the file names would sort naturally.


> > > - Sam Ruby
>
> - Sam Ruby
>
> > > On Tue, Jul 21, 2020 at 3:39 PM sebb <se...@gmail.com> wrote:
> > > >
> > > > The icla code attempts [1] to rename PDF files to icla.pdf and
> > > > icla.pdf.asc if there is a signature.
> > > >
> > > > However the code has been broken for a while, because the fileext is
> > > > nil unless the signature is empty [2]. Thus the condition becomes:
> > > >
> > > > if @signature.to_s.empty? or (not @signature.empty? && fileext != '.pdf')
> > > >
> > > > i.e. the condition is always true.
> > > >
> > > > AFAICT this has been the case since May 2017 [3]
> > > >
> > > > It's easy enough to fix (ensure fileext is always created).
> > > >
> > > > However I wonder if the rename is needed, and if so, why not rename
> > > > other files that have detached signatures? Removing the rename would
> > > > simplify the code, and make it easier to use the shared asvn_update
> > > > task code.
> > > >
> > > > Note that most applications seem to use icla.pdf anyway.
> > > >
> > > > Sebb
> > > >
> > > > [1] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L68
> > > >
> > > > [2] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L16
> > > >
> > > > [3] https://github.com/apache/whimsy/commit/786413d74a2515f91916225d929705b7b5c08811

Re: Renaming of ICLA PDF files - why?

[Sam Ruby <ru...@intertwingly.net>]

On Tue, Jul 21, 2020 at 5:46 PM sebb <se...@gmail.com> wrote:
>
> On Tue, 21 Jul 2020 at 21:31, Sam Ruby <ru...@intertwingly.net> wrote:
> >
> > The case that needs to be handled is if I were to send in an updated
> > ICLA by the name of 'sam-ruby.pdf'.  At that point, there would be a
> > sam-ruby.pdf in the documents/iclas directory, and one in the email,
> > and both would need to be placed into a directory.
>
> Indeed, but that's a separate issue.
> The code does check if there is a matching ICLA and will put out a
> warning if so.
> [The user must then either change the target name or use ICLA2 if the
> duplicate is intentional.]
>
> What I am referring to is the following:
>
> Assume there is an email with attachments named:
> abcd.pdf
> abcd.pdf.asc
>
> The code I am referring to will attempt to create the following files:
> iclas/subdir/icla.pdf
> iclas/subdir/icla.pdf.asc

My recollection is that the original intent was that the next icla
pdf/asc pair should be named icla2.pdf and icla2.pdf.asc.

And the reason for this naming convention was to both prevent the
naming conflict and to make it easier to determine the order in which
the documents were received as this would make it easier to find the
most recent document.

> That does not work currently.

The code could be fixed to do what I stated above.  Or it could be
replaced with a different approach.

> Even if it did, it would not fix the name clash issue.
> Also the code does not rename other file types such as .txt.

Possibly a second bug.

> So why the rename?

To prevent name clashes, and make it easier to identify the most
recent document or document pair.

> > - Sam Ruby

- Sam Ruby

> > On Tue, Jul 21, 2020 at 3:39 PM sebb <se...@gmail.com> wrote:
> > >
> > > The icla code attempts [1] to rename PDF files to icla.pdf and
> > > icla.pdf.asc if there is a signature.
> > >
> > > However the code has been broken for a while, because the fileext is
> > > nil unless the signature is empty [2]. Thus the condition becomes:
> > >
> > > if @signature.to_s.empty? or (not @signature.empty? && fileext != '.pdf')
> > >
> > > i.e. the condition is always true.
> > >
> > > AFAICT this has been the case since May 2017 [3]
> > >
> > > It's easy enough to fix (ensure fileext is always created).
> > >
> > > However I wonder if the rename is needed, and if so, why not rename
> > > other files that have detached signatures? Removing the rename would
> > > simplify the code, and make it easier to use the shared asvn_update
> > > task code.
> > >
> > > Note that most applications seem to use icla.pdf anyway.
> > >
> > > Sebb
> > >
> > > [1] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L68
> > >
> > > [2] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L16
> > >
> > > [3] https://github.com/apache/whimsy/commit/786413d74a2515f91916225d929705b7b5c08811

Re: Renaming of ICLA PDF files - why?

[sebb <se...@gmail.com>]

On Tue, 21 Jul 2020 at 21:31, Sam Ruby <ru...@intertwingly.net> wrote:
>
> The case that needs to be handled is if I were to send in an updated
> ICLA by the name of 'sam-ruby.pdf'.  At that point, there would be a
> sam-ruby.pdf in the documents/iclas directory, and one in the email,
> and both would need to be placed into a directory.

Indeed, but that's a separate issue.
The code does check if there is a matching ICLA and will put out a
warning if so.
[The user must then either change the target name or use ICLA2 if the
duplicate is intentional.]

What I am referring to is the following:

Assume there is an email with attachments named:
abcd.pdf
abcd.pdf.asc

The code I am referring to will attempt to create the following files:
iclas/subdir/icla.pdf
iclas/subdir/icla.pdf.asc

That does not work currently.
Even if it did, it would not fix the name clash issue.
Also the code does not rename other file types such as .txt.
So why the rename?

> - Sam Ruby
>
>
> On Tue, Jul 21, 2020 at 3:39 PM sebb <se...@gmail.com> wrote:
> >
> > The icla code attempts [1] to rename PDF files to icla.pdf and
> > icla.pdf.asc if there is a signature.
> >
> > However the code has been broken for a while, because the fileext is
> > nil unless the signature is empty [2]. Thus the condition becomes:
> >
> > if @signature.to_s.empty? or (not @signature.empty? && fileext != '.pdf')
> >
> > i.e. the condition is always true.
> >
> > AFAICT this has been the case since May 2017 [3]
> >
> > It's easy enough to fix (ensure fileext is always created).
> >
> > However I wonder if the rename is needed, and if so, why not rename
> > other files that have detached signatures? Removing the rename would
> > simplify the code, and make it easier to use the shared asvn_update
> > task code.
> >
> > Note that most applications seem to use icla.pdf anyway.
> >
> > Sebb
> >
> > [1] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L68
> >
> > [2] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L16
> >
> > [3] https://github.com/apache/whimsy/commit/786413d74a2515f91916225d929705b7b5c08811

Re: Renaming of ICLA PDF files - why?

[Sam Ruby <ru...@intertwingly.net>]

The case that needs to be handled is if I were to send in an updated
ICLA by the name of 'sam-ruby.pdf'.  At that point, there would be a
sam-ruby.pdf in the documents/iclas directory, and one in the email,
and both would need to be placed into a directory.

- Sam Ruby


On Tue, Jul 21, 2020 at 3:39 PM sebb <se...@gmail.com> wrote:
>
> The icla code attempts [1] to rename PDF files to icla.pdf and
> icla.pdf.asc if there is a signature.
>
> However the code has been broken for a while, because the fileext is
> nil unless the signature is empty [2]. Thus the condition becomes:
>
> if @signature.to_s.empty? or (not @signature.empty? && fileext != '.pdf')
>
> i.e. the condition is always true.
>
> AFAICT this has been the case since May 2017 [3]
>
> It's easy enough to fix (ensure fileext is always created).
>
> However I wonder if the rename is needed, and if so, why not rename
> other files that have detached signatures? Removing the rename would
> simplify the code, and make it easier to use the shared asvn_update
> task code.
>
> Note that most applications seem to use icla.pdf anyway.
>
> Sebb
>
> [1] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L68
>
> [2] https://github.com/apache/whimsy/blob/d714487b3da3f2efe7aed7021fa204eb22ba9cd5/www/secretary/workbench/views/actions/icla.json.rb#L16
>
> [3] https://github.com/apache/whimsy/commit/786413d74a2515f91916225d929705b7b5c08811