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.

Sign up for GitHub

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

Jump to bottom

Proposal: remove object from the language - Revisit #283

Merged
merged 1 commit into from
Feb 12, 2019
Merged

Proposal: remove object from the language - Revisit #283

merged 1 commit into from
Feb 12, 2019

Conversation

patmagee
Copy link
Member

@patmagee patmagee commented Feb 1, 2019

@EvanTheB I have attempted to rebase your PR #228. In this PR I have also removed the offending example which causes issues in the other PR

@cjllanwarne @geoffjentry can you please go over this with a fine toothed comb? The Rebase was quite ugly

@EvanTheB @patmagee
Remove object type
364037a 
Code review cjllanwarne
Clarify struct, correct some examples and simplify description.
Add struct serialization; copy of old object serialization
Regenerate toc
Re-introduce Object literal
@patmagee patmagee changed the title Proposal: remove object from the language - Revist Proposal: remove object from the language - Revisit Feb 1, 2019
@patmagee patmagee mentioned this pull request Feb 1, 2019
Closed
@EvanTheB
Copy link
Contributor

EvanTheB commented Feb 5, 2019

Apologies for the radio silence, lgtm

@cjllanwarne
Copy link
Contributor

@patmagee this looks correct to me! Thanks again for doing that rebase!

Let me know if this needs a re-vote or whether it goes immediately to "waiting for implementation". If the latter then I can rebase and merge my PR in Cromwell and this should be mergeable right afterwards.

@patmagee
Copy link
Member Author

@cjllanwarne This can move immediately to waiting for implementation and then merged oncve you merge yours. There are no changes introdced that were not in the original PR (other then removing the one example)

@patmagee patmagee added the awaiting implementation Changes that are awaiting implementation. label Feb 11, 2019
This was referenced Feb 11, 2019
Merged
Merged
@cjllanwarne
Copy link
Contributor

@patmagee just retesting this change in broadinstitute/cromwell#4443 now, since it needed a rebase on some grammar changes which occurred in the meantime. After that happens, I'll be able to declare this one "implemented"

@cjllanwarne
Copy link
Contributor

@patmagee implemented by broadinstitute/cromwell#4443 - so I think we can now:

@patmagee patmagee merged commit ca26ba3 into openwdl:master Feb 12, 2019
patmagee added a commit that referenced this pull request Feb 12, 2019
@patmagee
Merge pull request #273 from cjllanwarne/cjl_remove_object
4fc8986 
Related to #283 this has been implemented by cromwell
@patmagee patmagee added this to the v2.0 milestone Nov 20, 2019
@patmagee patmagee removed the awaiting implementation Changes that are awaiting implementation. label Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@patmagee @EvanTheB @cjllanwarne