Skip to content

Conversation

@rhinoella
Copy link
Contributor

Closes #503

Fixes current issues with GudPy tree name checking, enabling/disabling samples and related bugs.

@rhinoella rhinoella requested a review from trisyoungs May 1, 2024 13:48
@github-actions
Copy link

github-actions bot commented May 1, 2024

Test Results

  3 files    3 suites   40m 43s ⏱️
 92 tests  92 ✅ 0 💤 0 ❌
276 runs  276 ✅ 0 💤 0 ❌

Results for commit 6096e73.

♻️ This comment has been updated with latest results.

Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly your CI is failing on one of the OSX builds because of stray quote marks in the Gudrun source. I had the exact same thing on a Linux machine somewhere very recently and had to manually go and remove them (there are a couple of them)... migiht be something we need to address as a general issue.

return False
elif role == Qt.CheckStateRole and self.isSample(index):
if value == Qt.Checked:
if value == 2:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the fact that magic numbers like this are bad, I'm not sure I understand what's being checked for here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To check whether the check box is checked or unchecked

@rhinoella
Copy link
Contributor Author

Interestingly your CI is failing on one of the OSX builds because of stray quote marks in the Gudrun source. I had the exact same thing on a Linux machine somewhere very recently and had to manually go and remove them (there are a couple of them)... migiht be something we need to address as a general issue.

The issue is caused due to the github workflow update, amended in #506

@rhinoella rhinoella requested review from trisyoungs and removed request for trisyoungs May 3, 2024 15:24
Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd already approved this, but I'll approve it again!

@rhinoella rhinoella merged commit 855720e into develop May 10, 2024
@rhinoella rhinoella deleted the 503-fix-sample-import branch May 10, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GudPy sample import fails

3 participants