Terraform support #1882

derBroBro · 2017-10-09T19:55:03Z

What does this implement/fix? Explain your changes.

This adds support for terraform fmt.

Does this close any currently open issues?

Yes - #645

Any other comments?

Hope all fits.

npm run postinstall

Did not work.
As I am no fulltime dev it would be great if you can guide me what else I have to deliver.

Checklist

Check all those that are applicable and complete.

Merged with latest master branch
Regenerate documentation with npm run docs
Add change details to CHANGELOG.md under "Next" section
Added examples for testing to examples/ directory
Travis CI passes (Mac support)
AppVeyor passes (Windows support)

derBroBro · 2017-10-10T08:40:25Z

Travis failed because:

/usr/local/Homebrew/Library/Homebrew/brew.rb:12:in `<main>': Homebrew must be run under Ruby 2.3! (RuntimeError)

Have I to fix something or is this a pipeline thing?

Glavin001

Good start! Some small changes. We also need to see new examples for this language and ensure Travis CI and AppVeyor are passing. Thanks!

Glavin001 · 2017-10-10T13:18:54Z

CHANGELOG.md

@@ -1,4 +1,5 @@
 # Next
+- See [](). Add support for Terraform fmt.


Please fix the link:

See [#645](https://github.com/Glavin001/atom-beautify/issues/645). Add support for Terraform fmt.

Glavin001 · 2017-10-10T13:25:53Z

src/beautifiers/terraformfmt.coffee

+module.exports = class Terraformfmt extends Beautifier
+  name: "terraformfmt"
+  link: "https://www.terraform.io/docs/commands/fmt.html"
+  isPreInstalled: false


New beautifiers should use executables feature: #1687

Here's one Pull Request for example: https://github.com/Glavin001/atom-beautify/pull/1726/files#diff-4bf70ccc1cd2e9c4d2706b85b61fb41c

Please let me know if you have any questions. The Executables feature needs more documentation, so currently all we have are existing beautifiers are examples. See https://github.com/Glavin001/atom-beautify#beautifiers

If I change this, the exec is not found anymore. Testing with where (on windows) works.
Any Idea?

@Glavin001 now everything should be fixed.

Glavin001 · 2017-10-10T13:26:35Z

src/beautifiers/terraformfmt.coffee

+  }
+
+  beautify: (text, language, options) ->
+    @run("terraform", [


Use Executables feature.

# Before @run("terraform", [ # After @exe("terraform").run([

Glavin001 · 2017-10-10T13:27:27Z

src/beautifiers/terraformfmt.coffee

+  isPreInstalled: false
+
+  options: {
+    Terraform: true


Should be false indicating Language is supported with no/zero support for options.

Glavin001 · 2017-10-10T13:27:47Z

src/options.json

+      "tf"
+    ],
+    "properties": {
+      "indent_size": {


Need to fix Terraform: true above and rerun generate docs.

Glavin001 · 2017-10-10T13:30:31Z

npm run postinstall
Did not work.

I don't think there is a postinstall command: https://github.com/Glavin001/atom-beautify/blob/master/package.json#L412-L419

I think you meant npm run docs. What was the error?

derBroBro · 2017-10-10T14:35:19Z

For the question regarding postinstall I used this: https://github.com/Glavin001/atom-beautify/blob/master/docs/add-languages-and-beautifiers.md

Glavin001 · 2017-10-10T14:43:13Z

Oh thanks for letting me know! Looks like the docs are out of date. postinstall has been renamed to build-options and is run automatically with npm run docs:

    "build-options": "node script/build-options.js",
    "docs": "npm run build-options && coffee docs/",

Would you be able to change those docs in your Pull Request, too, please? Thanks!

derBroBro · 2017-10-11T17:56:44Z

I am not sure what I have to change to fix the issues. Can you support here ?

stevenzeck · 2017-10-11T18:34:36Z

Hi @derBroBro you need to have Terraform installed on both the AppVeyor (WIndows) and Travis CI (Linux and MacOS) build files for the tests to pass. .travis.yml for Travis and appveyor.yml for AppVeyor.

For Travis, you can use uncrustify as an example, as it looks like there is a Homebrew formula for Terraform: http://formulae.brew.sh/formula/terraform. Basically if the OS is OSX, brew install terraform otherwise docker pull <docker image here>.

derBroBro · 2017-10-11T20:58:20Z

Thank you, now appveyor looks ok, travis throws /usr/local/Homebrew/Library/Homebrew/brew.rb:12:in `<main>': Homebrew must be run under Ruby 2.3! (RuntimeError)

stevenzeck · 2017-10-11T21:06:46Z

There's an issue with Homebrew on Travis, uncomment line 95: https://github.com/Glavin001/atom-beautify/blob/master/.travis.yml#L95 and push it again.

derBroBro · 2017-10-12T10:58:02Z

Worked, I think with this the I changed everything.
The changes requested should be also fulfilled (not sure why not detected)

derBroBro · 2017-10-15T17:48:31Z

@Glavin001 anything I can to finish the PR?

derBroBro · 2017-10-26T14:49:33Z

Hello, any updates here?

stevenzeck · 2017-10-27T01:55:37Z

Looks good to me, I requested another review from Glavin.

Glavin001 · 2017-10-29T21:45:37Z

Reviewing now. I apologize, I have been unavailable to review Pull Requests lately. Let's see about getting this merged 😄 .

Glavin001

Looking great! Awesome work.

Last part is removing old isPreinstalled property from the beautifier and running npm run docs to make sure the docs are up-to-date -- I think they are, best to be sure.

Let's try to get this merged and into the next release! 😃

Glavin001 · 2017-10-29T21:53:31Z

src/beautifiers/terraformfmt.coffee

+module.exports = class Terraformfmt extends Beautifier
+  name: "terraformfmt"
+  link: "https://www.terraform.io/docs/commands/fmt.html"
+  isPreInstalled: false


isPreInstalled should be removed. See

atom-beautify/src/beautifiers/beautifier.coffee

Lines 40 to 41 in 49982c6

isPreInstalled: () ->

@executables.length is 0

Glavin001 · 2017-10-29T22:06:55Z

Looks like merge conflicts now, too. Once resolved, should be good to go!

Glavin001 · 2017-10-29T23:31:26Z

Awesome! Thank you for contributing!

Glavin001 · 2017-10-30T05:06:45Z

Published to v0.30.6

derBroBro added 6 commits October 9, 2017 21:39

add terraform

6088464

remove uneeded diffs

70ae361

remove uneeded diffs

e5a38e3

add changelog

b95ad76

run docs

cc08dbd

fix space

30d2264

Glavin001 self-requested a review October 10, 2017 13:17

Glavin001 self-assigned this Oct 10, 2017

Glavin001 added add-language add-beautifier labels Oct 10, 2017

Glavin001 requested changes Oct 10, 2017

View reviewed changes

derBroBro added 7 commits October 10, 2017 16:49

update options

34b09fd

update docs

0b3a8ba

update changelog

eeb65ea

rerun create docs

11f3571

add example

fcb15dc

update to run

91f7adb

remove space

cf74a98

add travis and appveyor requirements

b318eea

workarround for travis issue"

0d26638

stevenzeck requested a review from Glavin001 October 27, 2017 01:54

Glavin001 requested changes Oct 29, 2017

View reviewed changes

stevenzeck added 3 commits October 29, 2017 17:13

Remove isPreInstalled attribute

3ec28ea

Merge branch 'master' into terraform-support

11f54a3

Merge branch 'master' into terraform-support

df4ee70

Glavin001 approved these changes Oct 29, 2017

View reviewed changes

Glavin001 merged commit 9b9c1e5 into Glavin001:master Oct 29, 2017

Glavin001 added the pending-publication label Oct 29, 2017

Glavin001 added this to the v0.35.6 milestone Oct 30, 2017

Glavin001 added published and removed pending-publication labels Oct 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Terraform support #1882

Terraform support #1882

derBroBro commented Oct 9, 2017 •

edited

Loading

derBroBro commented Oct 10, 2017

Glavin001 left a comment

Glavin001 Oct 10, 2017

Glavin001 Oct 10, 2017

derBroBro Oct 10, 2017

derBroBro Oct 10, 2017

Glavin001 Oct 10, 2017

Glavin001 Oct 10, 2017

Glavin001 Oct 10, 2017

Glavin001 commented Oct 10, 2017

derBroBro commented Oct 10, 2017

Glavin001 commented Oct 10, 2017

derBroBro commented Oct 11, 2017

stevenzeck commented Oct 11, 2017 •

edited

Loading

derBroBro commented Oct 11, 2017

stevenzeck commented Oct 11, 2017

derBroBro commented Oct 12, 2017

derBroBro commented Oct 15, 2017

derBroBro commented Oct 26, 2017

stevenzeck commented Oct 27, 2017

Glavin001 commented Oct 29, 2017

Glavin001 left a comment

Glavin001 Oct 29, 2017

Glavin001 commented Oct 29, 2017

Glavin001 commented Oct 29, 2017

Glavin001 commented Oct 30, 2017

		@@ -1,4 +1,5 @@
		# Next
		- See [](). Add support for Terraform fmt.

Terraform support #1882

Terraform support #1882

Conversation

derBroBro commented Oct 9, 2017 • edited Loading

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any other comments?

Checklist

derBroBro commented Oct 10, 2017

Glavin001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Glavin001 commented Oct 10, 2017

derBroBro commented Oct 10, 2017

Glavin001 commented Oct 10, 2017

derBroBro commented Oct 11, 2017

stevenzeck commented Oct 11, 2017 • edited Loading

derBroBro commented Oct 11, 2017

stevenzeck commented Oct 11, 2017

derBroBro commented Oct 12, 2017

derBroBro commented Oct 15, 2017

derBroBro commented Oct 26, 2017

stevenzeck commented Oct 27, 2017

Glavin001 commented Oct 29, 2017

Glavin001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Glavin001 commented Oct 29, 2017

Glavin001 commented Oct 29, 2017

Glavin001 commented Oct 30, 2017

derBroBro commented Oct 9, 2017 •

edited

Loading

stevenzeck commented Oct 11, 2017 •

edited

Loading