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

Adding ToUnit methods #76

Merged
merged 17 commits into from
Jul 26, 2022
Merged

Conversation

JosefTaylor
Copy link
Contributor

@JosefTaylor JosefTaylor commented Jul 6, 2022

@JosefTaylor JosefTaylor added the type:feature New capability or enhancement label Jul 6, 2022
@JosefTaylor JosefTaylor self-assigned this Jul 6, 2022
@JosefTaylor
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 6, 2022

@JosefTaylor to confirm, the following checks are now queued:

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

@JosefTaylor JosefTaylor requested review from epignatelli and michaelhoehn and removed request for epignatelli July 7, 2022 13:20
@JosefTaylor
Copy link
Contributor Author

@BHoMBot check compliance

@JosefTaylor
Copy link
Contributor Author

JosefTaylor commented Jul 12, 2022

For string matching, could also check if the string is one of the enum names, to reduce number of strings we have to add to the switch-case.
I'd also thought about converting other enums (such as Midas, XML, other toolkits which have units enums) to strings, but the code wasn't super clean so I haven't yet. Also, suspect those will want to be removed once this is merged.

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 13, 2022

@JosefTaylor 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

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 13, 2022

@JosefTaylor fix requested for project compliance.

The errors with the CSProject (.csproj) files have been recorded as annotations on the checks tab.

I will apply the fixes to every case detailed on the checks tab with the exception of any references to the target framework. I am unable to provide fixes to the Target Framework automatically, these will need to be performed manually. If you want to perform the fixes in a different manner please resolve this manually and rerun the check.

If you are happy for me to go ahead and perform this action, please reply with:

@BHoMBot fix project file ref. 7324564983

@JosefTaylor
Copy link
Contributor Author

@BHoMBot fix project file ref. 7324564983

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 13, 2022

@JosefTaylor I have queued up your request to fix the csproj file(s). There are 0 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 13, 2022

@JosefTaylor I am now going to fix the CSProject compliance in accordance with the annotations previously made.

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 13, 2022

@JosefTaylor to confirm I have now resolved the CSProject compliance issues and pushed a commit to this Pull Request.

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check project-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 13, 2022

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

  • check project-compliance

There are 5 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 21, 2022

@JosefTaylor to confirm I have now resolved the project compliance issues and pushed a commit to this Pull Request.

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check project-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 21, 2022

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

  • check project-compliance

There are 5 requests in the queue ahead of you.

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check project-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 21, 2022

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

  • check project-compliance

@JosefTaylor
Copy link
Contributor Author

JosefTaylor commented Jul 21, 2022

Hey @FraserGreenroyd, I'm also having some issues with Project Compliance- one, it wants me on the csproj new format, which I'd love to have some help with, and two, it's complaining about the post-build, which I believe is OK- it might just be there's some white space it didn't understand.
EDIT: no sooner than I'd sent this did project-compliance pass- I wouldn't mind updating to the new format, but for now I'll run the required checks and ask for reviews.

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check-required

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 21, 2022

@JosefTaylor sorry, I didn't understand.
Was that comment an instruction for me? If so, could you state again what check you would like me to do?
For a list of available instructions, please see this wiki page.

@JosefTaylor
Copy link
Contributor Author

@michaelhoehn @peterjamesnugent would you mind reviewing? there's a nice big test script in the folder!

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 21, 2022

@JosefTaylor 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

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 21, 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.

@FraserGreenroyd
Copy link
Contributor

@JosefTaylor no worries, I think there's some teething problems with the post build check, @vietle-bh has been having trouble too. I'll be looking into it soon!
For the new project style, we can do a small workshop on that maybe as a use-case later in the milestone with @travispotterBH and @alexissantella perhaps? 😄

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check copyright-compliance dataset-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 21, 2022

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

  • check copyright-compliance

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check dataset-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 21, 2022

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

  • check dataset-compliance

@JosefTaylor JosefTaylor requested a review from mchaf July 21, 2022 21:54
@vietle-bh
Copy link

the new project style

Hi @FraserGreenroyd , I would like to join the new project style's workshop too 😎

@emidio-piermarini
Copy link
Member

Reviewed and looks good for the ones which have had functionality

Copy link
Member

@emidio-piermarini emidio-piermarini left a comment

Choose a reason for hiding this comment

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

Looks good for the ones that have functionality already. Suggest that we raise an issue to get units which currently have not support done.

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 26, 2022

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

  • check ready-to-merge

@JosefTaylor
Copy link
Contributor Author

@FraserGreenroyd please merge if you're happy with it!

@JosefTaylor
Copy link
Contributor Author

JosefTaylor commented Jul 26, 2022

@emidio-piermarini agreed; it currently supports all the units we've done before with ToFoot(), ToKilopoundForce() etc., which I believe more than covers the QuantityType attributes we've implemented, but there are many more compound units which could be added!
This also leaves room for adding nicknames to units, which could be done by anyone as needed

@FraserGreenroyd FraserGreenroyd merged commit 5564d77 into main Jul 26, 2022
@FraserGreenroyd FraserGreenroyd deleted the Units_Engine-#51-add-ToUnit-and-FromUnit branch July 26, 2022 15:57
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.

Units_Engine: provide ToUnit( quantity, unit ) and FromUnit( quantity, unit)
6 participants