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

Changelog for checkedint #5192

Merged
merged 1 commit into from Feb 26, 2017
Merged

Conversation

andralex
Copy link
Member

$ make -C ../dlang.org -f posix.mak pending_changelog
make: Entering directory '/home/andrei/d/dlang.org'
This command will be available soon.
make: Leaving directory '/home/andrei/d/dlang.org'

@wilzbach

@wilzbach
Copy link
Member

This command will be available soon.

-> dlang/dlang.org#1549

@@ -0,0 +1,28 @@
New module `std.experimental.checkedint` defines `Checked`, a lightweight and
Copy link
Member

@wilzbach wilzbach Feb 24, 2017

Choose a reason for hiding this comment

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

The format is equal to git messages.

  • first line = title
  • empty line
  • long description

For more details see the other changelog files or the doc:
https://github.com/dlang/phobos/tree/master/changelog

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool, thanks. Changed.

@andralex
Copy link
Member Author

@burner looks good?

@wilzbach
Copy link
Member

@burner looks good?

As dlang/dlang.org#1549 hasn't been merged yet and it's a bit hard to manually preview, here's a screenshot:

http://imgur.com/a/Ha6lj

@burner
Copy link
Member

burner commented Feb 26, 2017

LGTM thx @wilzbach

@burner
Copy link
Member

burner commented Feb 26, 2017

Auto-merge toggled on

@wilzbach
Copy link
Member

thx @wilzbach

FWIW I didn't want to nag on this again as I thought it's easily seen in the picture. The changelog format is really, really simple (and equal to the behavior of git commit): the first line is the title, followed by an empty line and then the description (that's the reason why the title seems to be cut-off on the screenshot. It's not a bug!).
(I wanted to go with something more advanced like YAML, which would might have been better because it's more explicit ...)

@wilzbach
Copy link
Member

Auto-merge toggled on

And another FWIW the auto-merge-squash is especially made for cases like this one where you don't want to push all commits for a simple change, but squash them into one commit instead ...

@@ -0,0 +1,22 @@
New: $(REF Checked, std, experimental, checkedint), a lightweight and highly
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't put links in the subject/title line. The whole line is turned into a link to the body, and you can't have links within links. You can put the REF in the body.

@aG0aep6G
Copy link
Contributor

The changelog format is really, really simple (and equal to the behavior of git commit): the first line is the title, followed by an empty line and then the description (that's the reason why the title seems to be cut-off on the screenshot. It's not a bug!).

The changelog tool should enforce that there's a blank line after the title line. Or, alternatively, allow multiple lines for the title, until the first blank line.

@wilzbach
Copy link
Member

Auto-merge toggled off

@wilzbach
Copy link
Member

The changelog tool should enforce that there's a blank line after the title line.

Seems like we haven't been that consistent in the past either:

https://github.com/dlang/phobos/blob/master/changelog/std-utf-decodeBack.dd

(but the changelog tool ignored this violation softly)

Or, alternatively, allow multiple lines for the title, until the first blank line.

Well in case we really want to go in this direction: dlang/tools#220

@wilzbach
Copy link
Member

Auto-merge toggled off

@andralex: I fixed the changelog file manually (title in one line, no REF in the title) and force-pushed the PR, s.t. we can move forward with this.
While I was at it I also rebased the three commits into one.

Copy link
Member

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

LGTM (@burner: you forgot the approval again ;-))

@wilzbach wilzbach merged commit 77f1a90 into dlang:master Feb 26, 2017
@andralex
Copy link
Member Author

@wilzbach sorry and thanks for fixing that - I missed the REF part. Thanks for using your powers for good!

@wilzbach
Copy link
Member

The changelog tool should enforce that there's a blank line after the title line.
Seems like we haven't been that consistent in the past either:

-> Fixup & validator: #5199

@wilzbach
Copy link
Member

@burner as I realized that some bits of the improved GH process might be new, I tried to summarize all changes that came to my mind: http://forum.dlang.org/post/qoamackqqnkqrxkochdu@forum.dlang.org
I hope it's partially helpful.

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