[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #17744 [Tor]: Add quotes when comparing strings in configure script
#17744: Add quotes when comparing strings in configure script
-------------------------+------------------------------------
Reporter: cypherpunks | Owner: zerosion
Type: defect | Status: needs_revision
Priority: Medium | Milestone: Tor: 0.2.8.x-final
Component: Tor | Version: Tor: 0.2.7.5
Severity: Trivial | Resolution:
Keywords: easy | Actual Points:
Parent ID: | Points:
Sponsor: |
-------------------------+------------------------------------
Comment (by teor):
Replying to [comment:16 cypherpunks]:
> Replying to [comment:14 teor]:
> > I can't see the point of quoting or changing the quotes around
internal variables or internal string literals. These are the majority of
the changes in the patch, and I'm not sure if they gain us anything. They
need to be weighed against the risk that a misplaced quote breaks the
build on some obscure configuration.
> > Was there a reason for these changes?
> As stated in the ticket description, ticket:16651#comment:28 mentions a
case where internal variables included spaces and caused issues. This was
the reason for opening this ticket in order to prevent similar instances
from causing problems in the future.
I understand now. I'd be happy for the patch to be merged as-is.
> >
> > The issue with re-flowing the entire file is that it makes it hard to
tell when the last non-whitespace change was made to each line.
> I believe git has options to ignore whitespace changes so they do not
interfere when tracking down changes. Furthermore, the tor codebase has
numerous commits which purely fix whitespace problems to appease `make
check-spaces`. In these cases change traceability isn't taken into account
either so IMO we shouldn't start with it now.
Fair enough. Please make whitespace changes in a separate commit. (And
ideally on another ticket.)
If we can work out a way to keep the whitespace consistent, ideally using
make check-spaces, then that will help us ensure that the initial change
is correct, and that future changes don't re-introduce whitespace issues.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/17744#comment:17>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs