Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update cad_audit_check.py #2559

Merged
merged 4 commits into from
Feb 11, 2025
Merged

Update cad_audit_check.py #2559

merged 4 commits into from
Feb 11, 2025

Conversation

nasmovk
Copy link
Contributor

@nasmovk nasmovk commented Feb 3, 2025

Added condition to catch the case of a project with no CAD instances

Update cad_audit_check.py

Description

Added condition to catch the case of a project with no CAD instances
Copy link
Contributor

devloai bot commented Feb 3, 2025

Automatic code review is enabled for this repository. Reviewing this PR now.

Copy link
Contributor

@devloai devloai bot left a comment

Choose a reason for hiding this comment

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

I've reviewed the changes to handle projects with no CAD instances. The core functionality looks good, but I've suggested some improvements for robustness (explicit None handling), efficiency (removing redundant variable), and consistency (naming and formatting). These changes will help make the code more maintainable while preserving the intended functionality.

@nasmovk nasmovk requested a review from sanzoghenzo February 7, 2025 13:22
Copy link
Contributor

@sanzoghenzo sanzoghenzo left a comment

Choose a reason for hiding this comment

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

Tank you!

@nasmovk
Copy link
Contributor Author

nasmovk commented Feb 11, 2025

So I think my own patch-3 branch is OK.
Do I need to do anything push anything else or pull request ? — sorry, but I am still learning github

@sanzoghenzo sanzoghenzo merged commit a267ef5 into pyrevitlabs:develop Feb 11, 2025
@sanzoghenzo
Copy link
Contributor

Sorry, I forgot I have the ability to do the merge and I was waiting for @jmcouffin to do it 😅

You did everything right, thank you very much for taking the time to learn git and github!

Copy link
Contributor

📦 New work-in-progress (wip) builds are available for 5.0.0.25042+1512-wip

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.

3 participants