Skip to content

Make run plan parameters an argument#207

Merged
tpoliaw merged 1 commit intoDiamondLightSource:mainfrom
tpoliaw:param_arg
May 18, 2023
Merged

Make run plan parameters an argument#207
tpoliaw merged 1 commit intoDiamondLightSource:mainfrom
tpoliaw:param_arg

Conversation

@tpoliaw
Copy link
Copy Markdown
Contributor

@tpoliaw tpoliaw commented May 16, 2023

Instead of an option that is required in every call. If no parameters are passed,
default to empty dict.

Instead of an option that is required in every call. If no parameters are passed,
default to empty dict.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2023

Codecov Report

Merging #207 (00accfb) into main (aaccc3f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #207      +/-   ##
==========================================
+ Coverage   87.50%   87.51%   +0.01%     
==========================================
  Files          41       41              
  Lines        1120     1121       +1     
==========================================
+ Hits          980      981       +1     
  Misses        140      140              
Impacted Files Coverage Δ
src/blueapi/cli/cli.py 97.43% <100.00%> (+0.03%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown
Contributor

@joeshannon joeshannon left a comment

Choose a reason for hiding this comment

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

It looks like a sensible change to me.
Do we want to check if others agree?

@callumforrester
Copy link
Copy Markdown
Contributor

Definitely nicer, cheers

@tpoliaw tpoliaw merged commit 50e4627 into DiamondLightSource:main May 18, 2023
@tpoliaw tpoliaw deleted the param_arg branch May 18, 2023 09:19
@DominicOram DominicOram mentioned this pull request Aug 17, 2023
keithralphs pushed a commit that referenced this pull request Aug 29, 2023
Since #207 the `-p` is
not needed for the CLI, this removes it in the docs.

Co-authored-by: DiamondJoseph <53935796+DiamondJoseph@users.noreply.github.com>
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.

3 participants