libs-gui icon indicating copy to clipboard operation
libs-gui copied to clipboard

Improvements to existing NSDataLink code

Open gcasa opened this issue 2 years ago • 2 comments
trafficstars

These changes will implement the rest of NSDataLink/NSDataLinkManager. This class was not often used, but is relatively easy to implement. This class was rarely used, but should be easy to finish.

gcasa avatar Mar 17 '23 10:03 gcasa

Looking at the diffs, it seems to me that the changes are almost all cosmetic (improving ivar names and changing whitespace). I think it would make things far easier to review if there were separate changes:

  1. a small set of functional changes to be reviewed carefully for correct logic
  2. cosmetic changes, which can be passed very quickly

I generally try to do cosmetic stuff separately first.

rfm avatar Dec 22 '23 10:12 rfm

Looking at the diffs, it seems to me that the changes are almost all cosmetic (improving ivar names and changing whitespace). I think it would make things far easier to review if there were separate changes:

  1. a small set of functional changes to be reviewed carefully for correct logic
  2. cosmetic changes, which can be passed very quickly

I generally try to do cosmetic stuff separately first.

They are at this point. My original intention with this PR was to make the class actually work. I implemented what is there a long time ago, but got stuck on how to monitor changes to a file without polling it (which is terribly inefficient) so, I wanted to come back to it and see if I could find a way to make it functional. Sometimes I do the "cosmetic" stuff while I am thinking as it helps me think more cleanly. I don't know if that makes sense, but it's my process. ;)

Point taken. It would be understandable from the reviewer's point of view. When I did this I hadn't considered that.

gcasa avatar Dec 22 '23 16:12 gcasa