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

Add fmt command #171

Merged
merged 1 commit into from Jul 17, 2017

Conversation

Projects
None yet
6 participants
@krishicks
Contributor

krishicks commented May 31, 2017

This introduces fly fmt in the spirit of go fmt, which is useful for people that use yaml-patch to modify their Concourse pipelines (yaml-patch isn't aware of Concourse pipelines, only YAML).

It presently uses a little thing I wrote for yaml-patch to handle placeholders in the pipeline:

Because key: {{placeholder}} is invalid YAML we have to wrap the placeholder in quotes to make it valid yaml just to unmarshal it into an atc.Config. Later, we need to unwrap the quotes from the placeholder.

If it's a requirement I can pull that out into a separate thing so as to not have to bring in all of yaml-patch just for this.

This PR is more to get the conversation started and a review of the implementation going.

@krishicks

This comment has been minimized.

Show comment
Hide comment
@krishicks

krishicks May 31, 2017

Contributor

The concourse-ci/status failure is unrelated and randomly failing locally for me even without this commit.

Contributor

krishicks commented May 31, 2017

The concourse-ci/status failure is unrelated and randomly failing locally for me even without this commit.

@chendrix chendrix added the fly label Jun 28, 2017

@chendrix chendrix added this to the v3.3.x milestone Jul 5, 2017

Kris Hicks
@ebabani

This comment has been minimized.

Show comment
Hide comment
@ebabani

ebabani Jul 17, 2017

Contributor

fmt is adding missing fields with empty values:

Examples:

-          args: ["running do step 2"]
+          args:
+          - running do step 2
+          dir: ""
-          source: {repository: busybox}
+          source:
+            repository: busybox
+          params: {}
+groups: []
+resources: []
+resource_types: []

Was this done on purpose?

Contributor

ebabani commented Jul 17, 2017

fmt is adding missing fields with empty values:

Examples:

-          args: ["running do step 2"]
+          args:
+          - running do step 2
+          dir: ""
-          source: {repository: busybox}
+          source:
+            repository: busybox
+          params: {}
+groups: []
+resources: []
+resource_types: []

Was this done on purpose?

@krishicks

This comment has been minimized.

Show comment
Hide comment
@krishicks

krishicks Jul 17, 2017

Contributor
Contributor

krishicks commented Jul 17, 2017

@ebabani ebabani merged commit dd0a471 into concourse:master Jul 17, 2017

1 of 2 checks passed

concourse-ci/status Concourse CI build failure
Details
ci/pivotal-cla Thank you for signing the Contributor License Agreement!
Details
@vito

This comment has been minimized.

Show comment
Hide comment
@vito

vito Aug 9, 2017

Member

We're gonna rename this to format-pipeline so it matches the scheme of all the other commands. We'll add a short name, fp (which also fits our short name scheme).

Member

vito commented Aug 9, 2017

We're gonna rename this to format-pipeline so it matches the scheme of all the other commands. We'll add a short name, fp (which also fits our short name scheme).

@vito vito unassigned ebabani Aug 9, 2017

@krishicks

This comment has been minimized.

Show comment
Hide comment
@krishicks

krishicks Aug 9, 2017

Contributor

Sad but makes sense.

Contributor

krishicks commented Aug 9, 2017

Sad but makes sense.

@krishicks krishicks deleted the krishicks:add-fmt-command branch Aug 9, 2017

@vito

This comment has been minimized.

Show comment
Hide comment
@vito

vito Aug 9, 2017

Member

Well at least it lets us add format-task on the future. :P

Member

vito commented Aug 9, 2017

Well at least it lets us add format-task on the future. :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment