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

Fix #15 #16

Merged
merged 2 commits into from
Nov 2, 2018
Merged

Fix #15 #16

merged 2 commits into from
Nov 2, 2018

Conversation

csjall
Copy link
Collaborator

@csjall csjall commented Nov 2, 2018

[x] This contribution adheres to CONTRIBUTING.md.

What does this Pull Request accomplish?

Fix broken OpenVeriStandConnection.vi. We changed the utilities without changing the test case.

Why should this Pull Request be merged?

Bug fix

What testing has been done?

Added a test for opening/closing the VeriStand connection. All tests are green.

@csjall csjall added the bug Something isn't working label Nov 2, 2018
@csjall csjall requested a review from rtzoeller November 2, 2018 20:21
Copy link
Contributor

@rtzoeller rtzoeller left a comment

Choose a reason for hiding this comment

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

Ran the SEECD tests with this change and everything looked good in terms of functionality.

I noticed there is an inconsistency in the hierarchy regarding where the unit tests live and their naming. I don't have a preference either way, but we should at least be consistent.

Fix it, then ship it!

image

@csjall csjall merged commit 6f2fc9a into master Nov 2, 2018
@csjall csjall deleted the dev/systemDefinitionInformation branch November 2, 2018 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants