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

In-house parser #58

Merged
merged 11 commits into from
Sep 8, 2020
Merged

In-house parser #58

merged 11 commits into from
Sep 8, 2020

Conversation

edigaryev
Copy link
Contributor

@edigaryev edigaryev commented Sep 3, 2020

Some things that still clearly need to be done:

  • support only_if (this is currently the reason why the test suite fails)
  • pass environment where it's needed for expansion
  • support missing fields that are still needed for the CLI, the rest will be handled via Complete the parser #59.

Resolves #16.

@edigaryev
Copy link
Contributor Author

@fkorotkov's there are a few files that are technically still under MIT since I've taken them directly from keanu:

Do you consent to re-license the code in these files under MPL-2.0, so I can remove these lines?

@fkorotkov
Copy link
Contributor

I consent but with MIT I don't think you even need to ask 😅

pkg/parser/node/finder.go Outdated Show resolved Hide resolved
pkg/parser/node/finder.go Outdated Show resolved Hide resolved
pkg/parser/parseable/default.go Outdated Show resolved Hide resolved
pkg/parser/parser.go Outdated Show resolved Hide resolved
@edigaryev
Copy link
Contributor Author

support only_if (this is currently the reason why the test suite fails)

Implemented in 2beda35.

However, since some changes to the Gval package are still waiting to be upstreamed (PaesslerAG/gval#38), go.mod temporarily points to https://github.com/edigaryev/gval/tree/expose-base-pieces.

@fkorotkov
Copy link
Contributor

I think replacing the RPC parser with the local one right away seem like a bit too bold. What do you think about having a --parser flag with the default remote value and an option to set it to local? Also that way parser_test.go can validate that local parser returns the same results as the remote one.

@edigaryev edigaryev marked this pull request as ready for review September 8, 2020 14:41
@edigaryev edigaryev merged commit 0a5cc22 into master Sep 8, 2020
@edigaryev edigaryev deleted the parser branch September 8, 2020 16:26
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.

Parse YAML config on CLI side
2 participants