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.rose stem name is empty string #250

Merged
merged 2 commits into from
Oct 5, 2023

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Aug 24, 2023

Issue documented in code and PR.

Before -s=name would lead to project name being manually set to '' rather than using FCM to setup a project name. This is a correct from the point of view of optparse, but might seem surprising to users.

@dpmatthews tagged as product owner/understand why the change.
@MetRonnie as code review - IMO it's so trivial we shouldn't spend much time on it.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@wxtim wxtim force-pushed the fix.rose_stem_name_is_empty_string branch from 1b00e13 to d397564 Compare August 24, 2023 08:32
@wxtim wxtim requested review from dpmatthews and MetRonnie August 24, 2023 08:33
@wxtim wxtim self-assigned this Aug 24, 2023
@wxtim wxtim added this to the 1.3.1 milestone Aug 24, 2023
@wxtim wxtim added bug Something isn't working small labels Aug 24, 2023
Before `-s=name` would lead to project name being manually
set to '' rather than using FCM to setup a project name.
This is a correct from the point of view of optparse, but
might seem surprising to users.
@wxtim wxtim force-pushed the fix.rose_stem_name_is_empty_string branch from d397564 to cd4de39 Compare August 24, 2023 08:35
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

LGTM, haven't tested

tests/unit/test_rose_stem_units.py Outdated Show resolved Hide resolved
@oliver-sanders oliver-sanders merged commit 7697d23 into cylc:1.3.x Oct 5, 2023
@wxtim wxtim deleted the fix.rose_stem_name_is_empty_string branch October 5, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants