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

enhancement: Update string renderer to add empty lines only when commits are present. #153

Merged
merged 3 commits into from
Nov 30, 2019

Conversation

shrikanthkr
Copy link
Contributor

Fixes: #118

Fix markdown-renderer to add empty lines only when release commits are present. If there are no commits it returns an empty string.

@shrikanthkr shrikanthkr changed the title enhancement: Update string renderer to add empty lines only when commit are present. enhancement: Update string renderer to add empty lines only when commits are present. Oct 12, 2019
@Turbo87
Copy link
Contributor

Turbo87 commented Oct 12, 2019

can you add a test case for this or adjust an existing test case if one exists already?

@shrikanthkr
Copy link
Contributor Author

@Turbo87 Yeah sure. Am working on that.

@shrikanthkr
Copy link
Contributor Author

@Turbo87 Fixed the code as per the snapshot file which was used for testing.

@shrikanthkr
Copy link
Contributor Author

@Turbo87 Any suggestions on this?

@Turbo87
Copy link
Contributor

Turbo87 commented Nov 6, 2019

there might have been a misunderstanding. the tests were passing fine before your change and after your change so in the context of the tests nothing changed which makes it unclear what this PR actually does.

what I meant with my last comment, was to write a new test that would fail before your change and pass after your change. otherwise it would be very easy to regress on this new behavior.

@shrikanthkr
Copy link
Contributor Author

Uh! Got it. Lemme do that and add more details

@shrikanthkr
Copy link
Contributor Author

@Turbo87 Added regression snap shot and spec. Can you please take a look.

@Turbo87
Copy link
Contributor

Turbo87 commented Nov 30, 2019

@shrikanthkr I've rebased your branch and simplified the implementation a little bit. The new tests seem to be passing fine with this implementation too and it seems a bit more readable to me. Let me know if that suits your needs too.

@shrikanthkr
Copy link
Contributor Author

@Turbo87 Thanks for reviewing it. Looks good. 👍

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

Successfully merging this pull request may close these issues.

Remove trailing whitespace from output when there are no changes
2 participants