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

[1.0/development grammar.hgr]: Allow empty strings as static strings #238

Merged
merged 2 commits into from
Sep 28, 2018

Conversation

cjllanwarne
Copy link
Contributor

No description provided.

@cjllanwarne cjllanwarne changed the title [1.0/development grammer.hgr]: Allow empty strings in static strings [1.0/development grammer.hgr]: Allow empty strings as static strings Aug 1, 2018
@cjllanwarne cjllanwarne changed the title [1.0/development grammer.hgr]: Allow empty strings as static strings [1.0/development grammar.hgr]: Allow empty strings as static strings Aug 6, 2018
@geoffjentry
Copy link
Member

I don't think we should be retroactively changing things, even when they're incorrect.

@@ -108,6 +108,7 @@ grammar {

mode<awaiting_version_name> {
r'1.1-draft' -> :version_name %pop
r'biscayne' -> :version_name %pop
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that biscayne should be in this at all, unless you'd like to propose that as the official name for not-yet-1.1

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm happier with 'biscayne' than 1.1-draft. biscayne forever in /dev, real numbers in /!dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to advocate for something other than 1.1 (since who knows what the version number will be if any of the breaking changes make it in)

Copy link
Member

Choose a reason for hiding this comment

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

It's more that at the moment biscayne is meaningless in WDL-world

I agree that 1.1 is not the right value.

Copy link
Member

Choose a reason for hiding this comment

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

I dont really know what biscayne is.. afaik it's a cromwell thing. I do not think it belongs in this pr. I would probably move to a patch version in this instance rather then introduce some weird code name that makes no sense

@cjllanwarne
Copy link
Contributor Author

cjllanwarne commented Aug 9, 2018

I like the "don't change things retrospectively" rule generally but in this case I'm not happy with tying our hands and being unable to address silly typos and bugs at all until the next full version.

What if I split these grammar change off as a 1.0.1 version - no real changes to 1.0 allowed, only typos and bug fix changes allowed?

@cjllanwarne cjllanwarne force-pushed the cjl_empty_static_strings branch from fb65a73 to fd69083 Compare September 27, 2018 15:47
@geoffjentry
Copy link
Member

This is just a bug fix on the grammar file, merging

@geoffjentry geoffjentry merged commit c4d1523 into openwdl:master Sep 28, 2018
@patmagee patmagee added this to the v2.0 milestone Nov 20, 2019
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.

4 participants