Closed hansu closed 1 week ago
Didn't even check if any of these warnings was fixed in master. I ended up using the version from master where there was a conflict. Not sure how the Git history will be with all these merges though.
The file in question was more qtplasmac-materials.py
Commit in master https://github.com/LinuxCNC/linuxcnc/commit/7cb30f1164bb8647a440dcdd075f6c1b0a671389 did this change:
- elif line.startswith('Tool number'):
+ elif line.startswith('Tool\\ number'):
And the commit in 2.9 447e94a41015b812b8ffe458ba0ede1d5567c32a did this:
- elif line.startswith('Tool\ number'):
+ elif line.startswith('Tool number'):
So the question is - do we have/except backslashes in this file which is beeing read there?
Maybe @snowgoer540 should have a look at this since @phillc54 has stepped down.
Thanks for the heads up. I’ll try to make some time in the next day after work or two to take a look.
The file in question was more
qtplasmac-materials.py
Commit in master 7cb30f1 did this change:
- elif line.startswith('Tool number'): + elif line.startswith('Tool\\ number'):
And the commit in 2.9 447e94a did this:
- elif line.startswith('Tool\ number'): + elif line.startswith('Tool number'):
So the question is - do we have/except backslashes in this file which is beeing read there?
Yes. The change to 2.9 is incorrect and breaks functionality. I believe the change to master to be correct.
Thanks for looking into this :+1:
Then we should fix it again in 2.9 and merge then.
The other conflicts like mentioned in my first comment are more or less equal - which version is the preferred one?
("\\..."
vs r"\..."
)
I don’t remember the specific reason I chose double slash in master. Raw string might be better because it seems more purposeful to anyone who stumbles across it in the future?
Otherwise I can also see an argument to just make it match what’s already in master. I’m happy to do whichever.
Otherwise I can also see an argument to just make it match what’s already in master. I’m happy to do whichever.
I did it with r" "
to match the other changes of that commit. In the merge to master we can choose to keep the version from master.
Excellent, thank you for writing the commit. I merged it.
No need to keep the "\" in master, if you're pulling all of the other changes forward, I'd rather the commits just stay cohesive for the fix they were intended for. No worries there.
Ok merge done.
Thank you!
While merging 2.9 into master I get conflicts in three files.
For example data.py was modified in master: 6e90e5235f0a7bb2a4ccdeea41db785228891b74 as well as in 2.9: 447e94a41015b812b8ffe458ba0ede1d5567c32a
@phillc54 or @havardAasen can you please merge 2.9 into master as you guys knows better which version to keep ;-)