Add "add" command #47

ordepdev · 2019-11-09T01:31:02Z

The following sequence of commands:

changelog-tool new
changelog-tool unreleased -e
changelog-tool add fixed "We fixed some nasty bug." -e
changelog-tool add added "And we added some cool feature." -e
changelog-tool add changed "And changed some little things as well." -e
changelog-tool release 1.0.0

Will result in this nice changelog:

# Change Log

All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/) and [Keep a CHANGELOG](http://keepachangelog.com/).

## [1.0.0] - 2019-11-09

### Fixed

- We fixed some nasty bug.

### Added

- And we added some cool feature.

### Changed

- And changed some little things as well.

SeanTAllen · 2019-11-09T01:32:56Z

@ordepdev You checked in a binary, you'll want to remove that.

SeanTAllen · 2019-11-09T01:33:37Z

changelog-tool/changelog.pony

  fun string(): String iso^ =>
-    if heading == _unreleased_heading then


i don't think you meant to remove all of this.

Well, if we don't remove this, it will print the unrelease thing with an empty body, without the entries that we're adding. That's why I removed it 👍

Doesn't this break the "unreleased" command?

The "unreleased" release should always show the fixed, added, and changed sections, even if they are empty.

I suspect that this is the cause of the CI failure

Yeah!

## [unreleased] - unreleased ### Fixed ### Added ### Changed

It's yielding ## [unreleased] - unreleased and makes the test fail.

I'm struggling to reach a point where I can print the empty sections only from unlreleased versions.

Turns out that unreleased sections were being parsed as None, so I added an extra check for each section saying that if it's unreleased and the section is None, it will use the Section._empty(....) for printing.

Also, for the parse test that's failing, a changelog must now contain all sections in "unreleased" because it can no longer assume that the "unreleased" release is either empty or nonexistent. It may be best to create a new class, Unreleased that uses Section instead of (Section | None) as field types. This would be simpler than treating it as any other release at this point.

@ordepdev would you want to include the changes above in this PR? If not, I can do it after this PR is merged.

I would love to but I think it's better to let you do it and take a look at your changes after!

tests/main.pony

SeanTAllen · 2019-11-09T01:36:57Z

Looks like there is a failing test

SeanTAllen · 2019-11-09T01:37:57Z

Awesome first-time contribution @ordepdev!

Theodus

Thanks @ordepdev! Looks good other than some style-guide-related issues and @SeanTAllen's comments

changelog-tool/main.pony

changelog-tool/changelog.pony

Closes ponylang#34.

Theodus · 2019-11-09T19:07:36Z

changelog-tool/changelog.pony

+        changed as Section
+      else error
+      end
+    section.entries.push("- " + entry)


A newline should be added to the end of the entry if it doesn't already have one.

Should this be done by using "String.contains" and appending "\n" or is there other idiomatic way of doing it?

Theodus · 2019-11-09T19:13:54Z

changelog-tool/main.pony

+      edit_or_print(
+        filepath,
+        edit,
+        Changelog(parse(filepath, filename)?)?


Adding .> create_unreleased() here before add_entry will likely solve the problem. An unreleased section should be created anyway if someone is using this command.

Hmm, creating a new "unreleased" release creates new empty sections.

changelog-tool new changelog-tool unreleased -e changelog-tool add fixed "We fixed some nasty bug." -e changelog-tool add added "And we added some cool feature." -e changelog-tool add changed "And changed some little things as well." -e changelog-tool release 1.0.0

Currently, this works as expected. If I add that command only the last "changed" will be reflected in the file, right?

Yeah, the problem is that the "unreleased" release must contain all sections if it exists. Previously the string function would assume that it is empty if it exists. But this is no longer a reasonable assumption, so the sections must be created if they don't exist. That's why it may be useful to create a new class for unreleased that reflects the requirement that the sections cannot have type None.

Makes sense, make the types do the work instead.

changelog-tool/changelog.pony

Theodus

OK, looks good. I'll make a followup PR with some additional changes to how the "unreleased" release is treated.

ordepdev · 2019-11-09T20:13:54Z

Nice, thanks for all the help! Will look this closely for changes 🍻

SeanTAllen reviewed Nov 9, 2019

View reviewed changes

tests/main.pony Outdated Show resolved Hide resolved

ordepdev force-pushed the add-new-command branch from 461fb8c to e6835be Compare November 9, 2019 01:35

SeanTAllen requested a review from Theodus November 9, 2019 01:37

SeanTAllen added the changelog - added Automatically add "Added" CHANGELOG entry on merge label Nov 9, 2019

SeanTAllen changed the title ~~Add the new "add" command~~ Add "add" command Nov 9, 2019

Theodus suggested changes Nov 9, 2019

View reviewed changes

changelog-tool/main.pony Outdated Show resolved Hide resolved

changelog-tool/main.pony Outdated Show resolved Hide resolved

changelog-tool/main.pony Outdated Show resolved Hide resolved

changelog-tool/changelog.pony Outdated Show resolved Hide resolved

ordepdev and others added 2 commits November 9, 2019 15:52

Add the new "add" command

9bf07b0

Closes ponylang#34.

Make things shine

632f9af

ordepdev force-pushed the add-new-command branch from e6835be to 632f9af Compare November 9, 2019 18:17

Theodus reviewed Nov 9, 2019

View reviewed changes

Don't ignore empty unreleased sections, make the build pass.

2f34f3a

Theodus suggested changes Nov 9, 2019

View reviewed changes

changelog-tool/changelog.pony Outdated Show resolved Hide resolved

changelog-tool/changelog.pony Outdated Show resolved Hide resolved

Just. Read. The. Style. Guide.

443bb21

Theodus approved these changes Nov 9, 2019

View reviewed changes

Theodus merged commit cf0262a into ponylang:master Nov 9, 2019

github-actions bot pushed a commit that referenced this pull request Nov 9, 2019

Update CHANGELOG for PR #47 [skip ci]

ec22403

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "add" command #47

Add "add" command #47

ordepdev commented Nov 9, 2019 •

edited by Theodus

Loading

SeanTAllen commented Nov 9, 2019

SeanTAllen Nov 9, 2019

ordepdev Nov 9, 2019

SeanTAllen Nov 9, 2019

Theodus Nov 9, 2019

Theodus Nov 9, 2019

ordepdev Nov 9, 2019

ordepdev Nov 9, 2019

Theodus Nov 9, 2019

Theodus Nov 9, 2019 •

edited

Loading

ordepdev Nov 9, 2019

SeanTAllen commented Nov 9, 2019

SeanTAllen commented Nov 9, 2019

Theodus left a comment

Theodus Nov 9, 2019

ordepdev Nov 9, 2019

Theodus Nov 9, 2019

ordepdev Nov 9, 2019

Theodus Nov 9, 2019

ordepdev Nov 9, 2019

Theodus left a comment

ordepdev commented Nov 9, 2019

		fun string(): String iso^ =>
		if heading == _unreleased_heading then

Add "add" command #47

Add "add" command #47

Conversation

ordepdev commented Nov 9, 2019 • edited by Theodus Loading

SeanTAllen commented Nov 9, 2019

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

Theodus Nov 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeanTAllen commented Nov 9, 2019

SeanTAllen commented Nov 9, 2019

Theodus 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

Theodus left a comment

Choose a reason for hiding this comment

ordepdev commented Nov 9, 2019

ordepdev commented Nov 9, 2019 •

edited by Theodus

Loading

Theodus Nov 9, 2019 •

edited

Loading