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

adds support for ItemsPath and MaxConcurrency #30

Merged
merged 1 commit into from
Apr 18, 2020

Conversation

xentek
Copy link
Contributor

@xentek xentek commented Oct 31, 2019

Map states allow two optional fields, ItemsPath and MaxConcurrency, which were being incorrectly raised as errors when parsing state machines that made use of these fields.

  • updates the j2119 specification to allow for these fields (fixes
    statelint doesn't support all Map options #29)
  • cleans up warnings when building the gemspec by specifying the version
    of the j2119 gem in a more constrained manner.
  • updates Gemfile so that the version of j2119 agrred with the
    version in the gemspec
  • removed unnecessary $LOAD_PATH manipulation from all ruby files in the
    project (refs Remove duplicate linter definitions #20)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

`Map` states allow two optional fields, `ItemsPath` and `MaxConcurrency`, which were being incorrectly
raised as errors when parsing state machines that made use of these fields.

- updates the _j2119_ specification to allow for these fields (fixes
  awslabs#29)
- cleans up warnings when building the gemspec by specifying the version
  of the `j2119` gem in a more constrained manner.
- updates `Gemfile` so that the version of `j2119` agrred with the
  version in the `gemspec`
- removed unnecessary `$LOAD_PATH` manipulation from all ruby files in the
  project (refs awslabs#20)
@xentek
Copy link
Contributor Author

xentek commented Oct 31, 2019

@timbray By the way, the master branch on GitHub does not contain this version: https://rubygems.org/gems/statelint/versions/0.4.0, which was released by varrav (couldn't be sure of the GitHub username).

It looks like the only real change in v0.4.0 of statelint was depending on v0.4.0 of the j2119.

This PR used 0.4.0 as the base, even though my fork is compared against 0.3.0. To ensure continuity, and a cleaner history, you may want to commit 0.4.0 so that there is a commit that matches what was released on ruby gems.org, before merging this PR.

I bumped the version to 0.4.1 in this PR, which may not be desirable, to ensure that it would be distinct from other versions (since I will be using my fork until this PR is merged and released)

Also, if it wasn't clear, I incorporated the changes from #20, since it looked like that PR has been abandoned, but that you agreed with the changes in principle. In addition, I made a few other minor edits to improve the "hygiene" of the codebase. Everything was submitted as one commit (my typical style) but I realize now that you may have liked the option to cherry pick around these non-essential edits. If that is the case, I would be happy to close and re-submit a PR with each change in it's own commit.

Let me know!

@melalawi
Copy link
Collaborator

Looks good to me!

@RobertFischer
Copy link

Please merge!

@melalawi melalawi merged commit 0b0425f into awslabs:master Apr 18, 2020
@melalawi
Copy link
Collaborator

Tested locally & merged!

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