Skip to content

Update lakeshore drivers to conform with standard#4473

Merged
jenshnielsen merged 10 commits intomicrosoft:masterfrom
jenshnielsen:driver/lakeshore_cleanup
Aug 5, 2022
Merged

Update lakeshore drivers to conform with standard#4473
jenshnielsen merged 10 commits intomicrosoft:masterfrom
jenshnielsen:driver/lakeshore_cleanup

Conversation

@jenshnielsen
Copy link
Copy Markdown
Collaborator

@jenshnielsen jenshnielsen commented Aug 4, 2022

I could be convinced that model should not be part of the class names. What do you think?

I have copied the drivers rather than making the old ones aliases since there was a fair bit of renaming involved but imported
the channel/modul classes from the new locations under aliases to avoid too much duplication.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 4, 2022

Codecov Report

Merging #4473 (cbe1469) into master (44a442f) will decrease coverage by 0.06%.
The diff coverage is 58.45%.

@@            Coverage Diff             @@
##           master    #4473      +/-   ##
==========================================
- Coverage   68.06%   67.99%   -0.07%     
==========================================
  Files         292      295       +3     
  Lines       31320    31405      +85     
==========================================
+ Hits        21317    21353      +36     
- Misses      10003    10052      +49     

Copy link
Copy Markdown
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

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

I could be convinced that model should not be part of the class names. What do you think?

yeah... it's annoying, however it is part of the official name of the instrument, so if we want to follow our own convention, then we should. or we have to relax the conveniention in a clear way :)

I have copied the drivers rather than making the old ones aliases since there was a fair bit of renaming involved

sad, so much diplication :( and the tests then also need to be copied (or at least use the new drivers)

@jenshnielsen jenshnielsen force-pushed the driver/lakeshore_cleanup branch from c1f094b to 3030f85 Compare August 4, 2022 16:11
@jenshnielsen jenshnielsen force-pushed the driver/lakeshore_cleanup branch from 17581b4 to 0aa2752 Compare August 5, 2022 07:54
Copy link
Copy Markdown
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

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

lovely!

@jenshnielsen jenshnielsen enabled auto-merge August 5, 2022 08:28
@jenshnielsen jenshnielsen merged commit fa29c7b into microsoft:master Aug 5, 2022
@jenshnielsen jenshnielsen added this to the Public api rewrite milestone Aug 8, 2022
@jenshnielsen jenshnielsen added driver docs Related to docs improvements labels Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Related to docs improvements driver

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants