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

Remove Extra NewLines between Comments and Inputs/Outputs to fix MarkDownLint Warnings #66

Merged
merged 9 commits into from
Oct 3, 2018

Conversation

X-Guardian
Copy link
Contributor

@X-Guardian X-Guardian commented Sep 27, 2018

Prerequisites

Put an x into the box(es) that apply:

  • This pull request fixes a bug.
  • This pull request adds a feature.
  • This pull request introduces breaking change.

For more information, see the Contributing Guide.

Description

Currently, if a main.tf has comments and the module has inputs and/or outputs, an additional newline is added between the comments and the inputs/outputs in the markdown. This is reported by MarkDownLint as a warning:

MD012/no-multiple-blanks: Multiple consecutive blank lines [Expected: 1; Actual: 2]

This pull request removes those additional newlines.

Checklist

Put an x into all boxes that apply:

Tests

  • I have modified tests to cover my changes.
  • All tests pass when I run make test.

I've modified the markdown golden test files, but the markdown tests fail for me, but I think it is because I am running them on Windows and it is a \r\n issue. Can someone confirm?

Documentation

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Code Style

  • My code follows the code style of this project.

@@ -27,6 +27,9 @@ func Print(document *doc.Doc, settings settings.Settings) (string, error) {
}

if document.HasOutputs() {
if document.HasInputs() {
Copy link
Member

Choose a reason for hiding this comment

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

Please move this block down between the sorting and the call to printOutputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@metmajer
Copy link
Member

@X-Guardian thanks for providing this fix 👍 Have you run make test?

@X-Guardian
Copy link
Contributor Author

@metmajer, the markdown tests fail for me, but I think it is because I am running them on Windows and it is a \r\n issue. Can you confirm?

@metmajer
Copy link
Member

@X-Guardian I've run the tests on my machine and some tests fail. Good news, there's a pattern: the comment in main.tfs header includes an extra (unexpected) newline.

This reminds me that CircleCI should actually run a pipeline that shows the test execution to you in this pull request. I'll try to have this fixed.

@metmajer
Copy link
Member

@X-Guardian CircleCI should now run a pipeline when you add to this pull request. Want to give it a try?

@X-Guardian
Copy link
Contributor Author

Cheers @metmajer, I added another commit, but no CircleCI was triggered...

@metmajer
Copy link
Member

metmajer commented Oct 1, 2018

Thanks for the update @X-Guardian and your continued support. I've contacted to look into this since I don't have admin rights.

@X-Guardian
Copy link
Contributor Author

@metmajer, I wonder if your CircleCI change will only trigger on a new pull request? Shall I create one to test?

@metmajer
Copy link
Member

metmajer commented Oct 1, 2018

@X-Guardian I haven't thought about this, while I would strongly assume that the pipeline is triggered by a commit, it's worth a try. Thanks!

@metmajer
Copy link
Member

metmajer commented Oct 3, 2018

@X-Guardian we've been able to fix the missing CircleCI builds for pull requests in #71. If you take a look, you should now an indicator (a green check mark or a red cross) indicating the status of your commit. By clicking on the indicator, you'll see an overlay where you can inspect the stages of the build. Beware that, for looking at the tests, I had to scroll down in the overlay.

@X-Guardian
Copy link
Contributor Author

@metmajer, CircleCI now working on commits, and I've fixed all the spurious blank lines from the tests.

@metmajer
Copy link
Member

metmajer commented Oct 3, 2018

@X-Guardian thanks for staying on it. Appreciate your contribution!

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