Skip to content

Add support for ToRhino for NurbsCurve for Rhino7 #232

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

Merged

Conversation

IsakNaslundBh
Copy link
Contributor

Issues addressed by this PR

Closes #215

Changing it so that version is checked and version5 convert is called if version is 5. For versions above 5 the same methodology is used and is instead put in the main method. ToRhino6 method remove and versioned to the main method.

Test files

Changelog

Additional comments

Changing it so that version is checked and version5 convert is called if version is 5. For versions above 5 the same methodology is used and is instead put in the main method.
ToRhino6  method remove and versioned to the main method.
@IsakNaslundBh IsakNaslundBh added the type:feature New capability or enhancement label Nov 15, 2022
@IsakNaslundBh IsakNaslundBh self-assigned this Nov 15, 2022
@IsakNaslundBh
Copy link
Contributor Author

IsakNaslundBh commented Nov 15, 2022

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

Very minor code formatting changes.

Otherwise looks good on code review - code has moved from the deleted file to the ToRhino file which I think it clean and I like the implementation.

As the suggestions are minor and are for formatting @IsakNaslundBh I'm not going to wait for you to commit them - I'll do them now and trigger all the bot checks, then leave you to merge - if you're unhappy with my suggestions that I commit you can then change them before merge - hopefully you don't mind doing it this way, but as I suspect you'll be happy, I figure this is the quickest way to allow a merge so we can unblock the R7 roll out we were discussing 😄

@FraserGreenroyd
Copy link
Contributor

FraserGreenroyd commented Nov 15, 2022

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

@FraserGreenroyd to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

@FraserGreenroyd to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

There are 6 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 16, 2022

The check code-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 16, 2022

The check documentation-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 16, 2022

The check project-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 16, 2022

The check installer has already been run previously and recorded as a successful check. This check has not been run again at this time.

@IsakNaslundBh
Copy link
Contributor Author

Happy with the changes @FraserGreenroyd .

Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

Should have remembered to do this bit last night...

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 16, 2022

@IsakNaslundBh to confirm, the following actions are now queued:

  • check ready-to-merge

@IsakNaslundBh IsakNaslundBh merged commit 00f5812 into main Nov 16, 2022
@IsakNaslundBh IsakNaslundBh deleted the Rhinoceros_Toolkit-#215-AddSupportForNUrbsConvertForRhino7 branch November 16, 2022 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rhinoceros_Engine: Add support for converting BH.oM.Geometry.NurbsCurve ToRhino in Rhino 7
2 participants