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

Extract export package from gh #67

Merged
merged 3 commits into from Aug 17, 2022
Merged

Extract export package from gh #67

merged 3 commits into from Aug 17, 2022

Conversation

samcoe
Copy link
Contributor

@samcoe samcoe commented Aug 16, 2022

This PR extracts out the export package from gh. Functionality wise, there are minimal changes compared to the counterpart packages in gh. After this is merged I will make a follow up PR in cli/cli to use this extracted package.

cc cli/cli#5544

}
}

func (t *Template) Parse(tpl string) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only change from the cli version of this package. Parse now needs to be explicitly called rather than implicitly being called from within Execute. I made this change to reduce the number of arguments to New and Execute. Additionally, I find the explicitness of it to be easier to follow.

@samcoe samcoe changed the title Templates Extract export package from gh Aug 16, 2022
@samcoe samcoe marked this pull request as ready for review August 16, 2022 15:00
@samcoe samcoe requested a review from vilmibm August 16, 2022 15:00
)

// Evaluate a jq expression against an input and write it to an output.
func Evaluate(input io.Reader, output io.Writer, expr string) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open to naming suggestions here? I also changed the argument order to have the input come first compared to the same function in gh.

Copy link
Contributor

Choose a reason for hiding this comment

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

naming makes sense to me

return t.tmpl.Execute(t.output, data)
}

func (t *Template) Flush() error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to be called End but I felt Flush was a more clear representation of what it does.

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

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

looks great, thanks for being clear about what changed.

)

// Evaluate a jq expression against an input and write it to an output.
func Evaluate(input io.Reader, output io.Writer, expr string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

naming makes sense to me

@samcoe samcoe enabled auto-merge (squash) August 17, 2022 12:25
@samcoe samcoe merged commit 3630ab3 into trunk Aug 17, 2022
@samcoe samcoe deleted the templates branch August 17, 2022 12:29
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.

None yet

2 participants