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

Add --json and --json-file CLI args to Lfc #1637

Closed
wants to merge 17 commits into from
Closed

Add --json and --json-file CLI args to Lfc #1637

wants to merge 17 commits into from

Conversation

patilatharva
Copy link
Collaborator

@patilatharva patilatharva commented Mar 10, 2023 •
edited
Loading

To be deleted - moved PR to #1686.

@patilatharva patilatharva linked an issue Mar 10, 2023 that may be closed by this pull request
Add --json CLI arg to Lfc #1620
Closed
@patilatharva
Copy link
Collaborator Author

@lhstrh this PR might take another week or so - the functionality is implemented but I can't write rigorous tests for the --json and --json-file options without building on rigorous tests for the other CLI options (i.e. the ones unpacked from JSON). So I'll be working on the latter first: #1641.

@tanneberger
Copy link
Member

tanneberger commented Mar 14, 2023 •
edited
Loading

OK the let me try putting it together with lingo

@patilatharva
Copy link
Collaborator Author

@revol-xut thanks, let me know if anything breaks!

@patilatharva patilatharva marked this pull request as ready for review March 21, 2023 17:40
@patilatharva patilatharva requested a review from lhstrh March 21, 2023 17:41
@patilatharva
Copy link
Collaborator Author

@lhstrh this PR is ready but its file diff is huge because it also contains changes from the CLI test PR #1668, so maybe we should merge that first.

lhstrh
lhstrh reviewed Mar 21, 2023
View reviewed changes
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

I don't think there should be any changes to the reactor-rs and reactor-c submodule in this PR...

@patilatharva
Copy link
Collaborator Author

I don't think there should be any changes to the reactor-rs and reactor-c submodule in this PR...

Not sure how those ended up there...I'll make sure they're reverted.

@lhstrh
Copy link
Member

lhstrh commented Apr 4, 2023

@patilatharva, after merging your other PR, there are some conflicts with this branch. Could you please fix these? After that, we should be able to merge PR as well...

@cmnrd cmnrd deleted the cli-json branch June 8, 2023 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhstrh lhstrh lhstrh left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add --json CLI arg to Lfc
3 participants
@patilatharva @tanneberger @lhstrh