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 port.access_labels #1032

Merged
merged 5 commits into from
May 27, 2022
Merged

Conversation

daico007
Copy link
Member

PR Summary:

Address #1031, add extra check for port access labels from its root compound. Change port.access_labels to be set to avoid duplicated labels.

PR Checklist


  • Includes appropriate unit test(s)
  • Appropriate docstring(s) are added/updated
  • Code is (approximately) PEP8 compliant
  • Issue(s) raised/addressed?

Copy link
Contributor

@CalCraven CalCraven left a comment

Choose a reason for hiding this comment

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

Probably just good to add a test similar to what you have in issue #1031 that validates all the port labels are accessible.

@codecov
Copy link

codecov bot commented May 3, 2022

Codecov Report

Merging #1032 (482fef1) into main (bcbf980) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1032   +/-   ##
=======================================
  Coverage   90.36%   90.37%           
=======================================
  Files          64       64           
  Lines        9011     9014    +3     
=======================================
+ Hits         8143     8146    +3     
  Misses        868      868           
Impacted Files Coverage Δ
mbuild/port.py 98.86% <100.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcbf980...482fef1. Read the comment docs.

Copy link
Contributor

@CalCraven CalCraven left a comment

Choose a reason for hiding this comment

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

Nice fix!

@daico007 daico007 merged commit f90c448 into mosdef-hub:main May 27, 2022
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.

None yet

2 participants