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

Upgrade to BuildingSync Version 2.5 #21

Merged
merged 5 commits into from
Sep 29, 2023
Merged

Conversation

JieXiong9119
Copy link
Contributor

No description provided.

@JieXiong9119 JieXiong9119 added the enhancement New feature or request label Sep 12, 2023
@JieXiong9119 JieXiong9119 self-assigned this Sep 12, 2023
@JieXiong9119
Copy link
Contributor Author

This has to wait until we release BuildingSync v2.5.
More package release work needs to be done.

bsyncpy/bsync.py Outdated
"Associated Air Balance Council (AABC) Certified Member Agency",
"Associated Air Balance Council (AABC) Test and Balance Technician",
"Association of Energy Engineers Certified Building Commissioning Firm Program (CBCF)",

Choose a reason for hiding this comment

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

Suggested change
"Association of Energy Engineers Certified Building Commissioning Firm Program (CBCF)",
"Association of Energy Engineers Certified Building Commissioning Firm (CBCF)",

Choose a reason for hiding this comment

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

@ThibaultMarzullo, Aside from the redundant whitespace after "Firm", I agree with the suggested change.

Choose a reason for hiding this comment

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

Suggested change
"Association of Energy Engineers Certified Building Commissioning Firm Program (CBCF)",
"Association of Energy Engineers Certified Building Commissioning Firm (CBCF)",

Thank you Mark

bsyncpy/bsync.py Show resolved Hide resolved
bsyncpy/bsync.py Show resolved Hide resolved
bsyncpy/bsync.py Show resolved Hide resolved
bsyncpy/bsync.py Outdated Show resolved Hide resolved
bsyncpy/bsync.py Outdated
"International Union of Operating Engineers Certified Energy Specialist",
"MEP Professional Engineer",

Choose a reason for hiding this comment

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

Suggested change
"MEP Professional Engineer",
"Mechanical, Electrical and Plumbing (MEP) Professional Engineer (PE)",

Choose a reason for hiding this comment

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

@ThibaultMarzullo, I agree with the suggested change.

bsyncpy/bsync.py Outdated
"International Union of Operating Engineers Certified Energy Specialist",
"MEP Professional Engineer",
"NYSERDA FlexTech Consultant",

Choose a reason for hiding this comment

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

Suggested change
"NYSERDA FlexTech Consultant",
"New York State Energy Research and Development Authority (NYSERDA) FlexTech Consultant",

Choose a reason for hiding this comment

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

@ThibaultMarzullo, I agree with the suggested change.

bsyncpy/bsync.py Show resolved Hide resolved
bsyncpy/bsync.py Show resolved Hide resolved
@@ -281,7 +281,7 @@ def do_simpleType(element) -> BSElement:
if union_type.startswith("auc:"):
bs_element.element_union.append(union_type[4:])
else:
raise RuntimError(f"union out of scope: {union_type}")
raise RuntimeError(f"union out of scope: {union_type}")

Choose a reason for hiding this comment

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

good catch

@ThibaultMarzullo
Copy link

Some of the suggestions above might have to be reflected in the schema.

Copy link
Contributor

@haneslinger haneslinger left a comment

Choose a reason for hiding this comment

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

Looking good, but you're missing bumping the version in pyproject.toml, as mentioned in the "Updating Version" section of READ.ME

Also, "2.4" appear several times in the READ.ME

@JieXiong9119
Copy link
Contributor Author

JieXiong9119 commented Sep 20, 2023

@markborkum @ThibaultMarzullo The comments related to the enumerations under AuditorQualificationType should go to the schema repo, but we can discuss and work around here first. These enumerations are changes coming from this PR and this PR.

@haneslinger
Copy link
Contributor

looking good! a few stray "2.4" in tests/test_basic.py

@JieXiong9119
Copy link
Contributor Author

@ThibaultMarzullo It seems there are just a few modifications on the enumerations. Do we want to include the changes in 2.5 release? We may need to confirm with @nllong if these are counted as breaking changes or not.

@ThibaultMarzullo
Copy link

Thank you @haneslinger and @markborkum for your review! @JieXiong9119 feel free to include in 2.5 if we are still on time. Thank you for updating this!

version & new schema change for 2.5
@haneslinger haneslinger merged commit 1289fa4 into develop Sep 29, 2023
@haneslinger haneslinger deleted the feat/support-bsync-2.5 branch September 29, 2023 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants