Skip to content

add changelog from directory builder#186

Merged
MartinNowak merged 5 commits intodlang:masterfrom
wilzbach:add_changelog_dir_builder
Jan 6, 2017
Merged

add changelog from directory builder#186
MartinNowak merged 5 commits intodlang:masterfrom
wilzbach:add_changelog_dir_builder

Conversation

@wilzbach
Copy link
Contributor

As discussed in the phobos PR this adds the changelog builder to the tools repo.
I also modified the changed tool to accept an additional output flag.

@CyberShadow
Copy link
Member

So how does this change http://wiki.dlang.org/DMD_Release_Building#Build_steps ? I don't understand how the changes in changed.d correlate with the new program.

@wilzbach wilzbach force-pushed the add_changelog_dir_builder branch from 8ed255f to cef6a79 Compare April 28, 2016 22:40
@wilzbach
Copy link
Contributor Author

copy the changelog.dd parts from the dmd/druntime/phobos stable repos to a new dlang.org/changelog/2.068.1.dd

Would be sth. like run rdmd ../phobos/changelog > changelog/2.072.0.dd (or maybe even a Makefile target).

I don't understand how the changes in changed.d correlate with the new program.

Currently the bug issues just gets copy-pastied to the bottom - see e.g. http://dlang.org/changelog/2.071.0.html

I updated the script to build from one file only to add the relative links between the entries - it will be rather easy (just two lines) to change it to "pure github" messages.
I will wait for @MartinNowak for further input & his ideas now ;-)

@MartinNowak
Copy link
Member

Would be nicer to integrate both tools.

@wilzbach
Copy link
Contributor Author

wilzbach commented May 1, 2016

Would be nicer to integrate both tools.

Sure, but please don't forget the people who just want to test their changelog - they don't need to download all bugzilla fixes.
So

  1. extra flag to disable bugzilla
  2. extra flag to enable bugzilla
  3. seperate scripts

@CyberShadow
Copy link
Member

they don't need to download all bugzilla fixes

Is that a problem? I've never run changed.

The goal is to run it as part of the dlang.org build anyway, to preview the upcoming changelog.

@wilzbach
Copy link
Contributor Author

wilzbach commented May 2, 2016

Is that a problem? I've never run changed.

The idea was to make the bugzilla part optional/toggleable, s.t.

  1. you (=Phobos contributor) don't have to wait for the changed script to finish (and don't need a working internet connection)
  2. you don't need to have every git repo from dlang cloned (e.g. installer)

@CyberShadow
Copy link
Member

Yep, it needs to be moved out of the installer repository if it is to be used in day-to-day builds.

How long does the bugzilla query take?

@wilzbach
Copy link
Contributor Author

wilzbach commented May 2, 2016

How long does the bugzilla query take?

Script takes about 15s to run.

@MartinNowak
Copy link
Member

Yep, it needs to be moved out of the installer repository if it is to be used in day-to-day builds.

How long does the bugzilla query take?

Could be cached, but the query itself should already be very efficient (just querying some rows by primary key).

@CyberShadow
Copy link
Member

Yeah, 15 seconds isn't too bad.

I suggest merging it with changed then.

Yep, it needs to be moved out of the installer repository if it is to be used in day-to-day builds.

I had misunderstood where the "installer" requirement came from. Making the git/Bugzilla query optional would resolve it.

@wilzbach wilzbach force-pushed the add_changelog_dir_builder branch from cef6a79 to 0466196 Compare October 7, 2016 14:05
@wilzbach wilzbach force-pushed the add_changelog_dir_builder branch from 0466196 to a18ebc0 Compare October 7, 2016 17:05
It stores its result in DDoc form to a text file.

Copyright: Dmitry Olshansky 2013.
Copyright: D Language Foundation 2016.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DmitryOlshansky are you okay with moving the copyright to the D Language Foundation?

Copy link
Member

Choose a reason for hiding this comment

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

Sure whatever. When I started this script it was a brutal hack to show that automation is possible and easy.

It is also possible to directly preview the generated changelog file:

---
rdmd changed.d --start=2013-01-01 --end=2013-04-01
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't supported anymore.

std.getopt.config.passThrough,
"output|o", &outputFile,
"date", &nextVersionDate,
"version", &nextVersionString,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MartinNowak is passing a CLI flag okay for you?
(Otherwise the new version is only written once in the file - at the Macro section)

Copy link
Member

Choose a reason for hiding this comment

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

Sure, already have the version everywhere.

changed.d Outdated
if (args.length != 2)
auto outputFile = "./changelog.dd";
auto nextVersionString = "LATEST";
auto nextVersionDate = "September 19, 2016";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a nice way to get this with Vanilla D? I couldn't find one :/

Copy link
Member

Choose a reason for hiding this comment

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

Not really.

@wilzbach
Copy link
Contributor Author

wilzbach commented Oct 7, 2016

I suggest merging it with changed then.

Ok I finally got around doing this. It now checks the dmd, druntime and phobos for manual text changelog files in the discussed format (simple Git commit style with the title as single line, followed by a separating empty line and then the detailed description).

I had misunderstood where the "installer" requirement came from. Making the git/Bugzilla query optional would resolve it.

If no arguments are passed, Bugzilla isn't queried (but a friendly message about this is shown).

@CyberShadow If you still want to integrate the pending changelog into DAutoTest something like this will work then:

rdmd changed.d "v2.071.2..upstream/stable" && dmd ../dlang.org/macros.ddoc ../dlang.org/html.ddoc ../dlang.org/dlang.org.ddoc ../dlang.org/doc.ddoc ../dlang.org/changelog/changelog.ddoc changelog.dd -Df../dlang.org/changelog/pending.html

@CyberShadow
Copy link
Member

@CyberShadow If you still want to integrate the pending changelog into DAutoTest something like this will work then:

Can this be a dlang.org Makefile target, part of the default target?

@wilzbach
Copy link
Contributor Author

wilzbach commented Oct 7, 2016

Can this be a dlang.org Makefile target, part of the default target?

Yes of course :)
But let's go step by step and first agree on the changelog building here.

Copy link
Member

@MartinNowak MartinNowak left a comment

Choose a reason for hiding this comment

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

Looks mostly good.
Can you summarize the changelog format again here for reference? It was ddoc with meta header, correct?

changedRepos.each!(x => x.changes.writeTextChangesHeader(w, x.headline));

if (revRange.length)
w.put("$(BR)$(BIG $(RELATIVE_LINK2 bugfix-list, List of all bug fixes and enhancements in D $(VER).))\n\n");
Copy link
Member

Choose a reason for hiding this comment

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

👏 having to add $(VER) myself everytime, started to become really annoying.

changed.d Outdated
// print the overview headers
changedRepos.each!(x => x.changes.writeTextChangesHeader(w, x.headline));

if (revRange.length)
Copy link
Member

Choose a reason for hiding this comment

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

revRange.length reads confusing (range + length), how about !revRange.empty?

revRange.getBugzillaChanges.writeBugzillaChanges(w);
}

w.formattedWrite("$(CHANGELOG_NAV_LAST %s)\n", previousVersion);
Copy link
Member

Choose a reason for hiding this comment

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

There is a small script to fix all the navigation.
https://github.com/dlang/dlang.org/blob/cb4129cb40b430ba5fcb508a8fe2f87fa9fbc2ac/changelog/update_nav.sh
Don't need to fully integrate that functionality for now though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok let's go with the most simple, working version for now and then continue to extend it :)

changed.d Outdated
{
// search for raw change files
alias Repo = Tuple!(string, "path", string, "headline");
auto repos = [Repo("dmd", "Language changes"),
Copy link
Member

Choose a reason for hiding this comment

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

We had a distinction for compiler and language changes.
Examples for noteworthy compiler changes would be a huge speedup or fixed template emission.
Any idea how to support those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well initially I suggested to use sth. like YAML for the entries, then we could have our own attribute and use sth. like:

title: Foo Bar
category: language-change
priority: 4
description: >
a long description

Now we already have two attributes category and priority, so I am seriously reconsidering to use YAML. Any opinions on this @MartinNowak @CyberShadow?

bool hideTextChanges = false;
string revRange;

auto helpInformation = getopt(
Copy link
Member

@MartinNowak MartinNowak Oct 8, 2016

Choose a reason for hiding this comment

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

Please use and provide option help.
See https://dlang.org/phobos/std_getopt.html#.getopt.

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 - see below)

changed.d Outdated
.map!(repo => Repo(buildPath("..", repo.path, "changelog"), repo.headline))
.filter!(x => x.path.exists)
.map!(x => tuple!("headline", "changes")(x.headline, x.path.readTextChanges.array))
.filter!(x => !x.changes.empty);
Copy link
Member

@MartinNowak MartinNowak Oct 8, 2016

Choose a reason for hiding this comment

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

Please keep a literal and understandable lambda argument name instead of using x, r for repo would be fine.
Can live w/ the x, but missing names are usually a code smell, b/c it requires the code reader to extract a mental model from raw code, instead of having the writer present one.

changed.d Outdated
ChangelogEntry entry = {
title: (cast(string) fileRead[0..firstLineBreak]).strip,
description: (cast(string) fileRead[firstLineBreak..$]).strip,
basename: filename.baseName.stripExtension // let's be safe here
Copy link
Member

Choose a reason for hiding this comment

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

What does safe mean here? Remove comment?

changed.d Outdated
break;

case "enhancement":
case "enhancement":
Copy link
Member

Choose a reason for hiding this comment

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

wrong formatting

@MartinNowak
Copy link
Member

One design problem came to my mind. There is no defined order/priority of changelog entries, right now it will just use the folders default order (which is alphabetically on my OS+FS, not sure about others).

We can leave this as a final manual polish step for now.

For the future we might adopt some P1_20161231235959_slug-name.dd scheme. With P1-P5 matching Bugzilla's highest to least priority system. It's a bit too complex to begin with and our changelogs shouldn't be that big in the beginning.

@MartinNowak
Copy link
Member

MartinNowak commented Oct 10, 2016

For the future we might adopt some P1_20161231235959_slug-name.dd scheme. With P1-P5 matching Bugzilla's highest to least priority system. It's a bit too complex to begin with and our changelogs shouldn't be that big in the beginning.

Let's adopt a color/font-style scheme for the changelog priority instead.
Something for more/less important stuff, and red for deprecations/breakages.
I'll go over the giant 2.072.0 changelog to prioritze stuff and standardize this.

@MartinNowak
Copy link
Member

Ping, any interest to continue the work on this? Didn't found time for the prioritization, feel free to propose any scheme.

}

return result.data;
string toString(Month month){
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While not necessary, hardcoding the Date seemed lame. Unfortunately Phobos doesn't know anything about the mapping of Dates to Month (at least I couldn't find anything).

@wilzbach
Copy link
Contributor Author

Ping, any interest to continue the work on this? Didn't found time for the prioritization, feel free to propose any scheme.

Thanks for the ping.

Can you summarize the changelog format again here for reference? It was ddoc with meta header, correct?

Added it to the header of the file too.
Yep, it is similar to the git commit format - an entry consists of a title line, a blank separator line and the description, e.g.

Title

My awesome description

Let's adopt a color/font-style scheme for the changelog priority instead.
Something for more/less important stuff, and red for deprecations/breakages.
..
For the future we might adopt some P1_20161231235959_slug-name.dd scheme. With P1-P5 matching Bugzilla's highest to least priority system. It's a bit too complex to begin with and our changelogs shouldn't be that big in the beginning.

I already commented about this in a comment as it seems to me that we need some meta information per entry, so using a slightly "more complex" file format might be better, e.g. YAML is pretty easy to write & parse:

title: An ugly bug
category: language-change
priority: 5
description: >
Write your long description here

We can leave this as a final manual polish step for now.

Yep let's go step by step and go with the simplest working version and then extend it gradually :)

@wilzbach
Copy link
Contributor Author

wilzbach commented Dec 9, 2016

Ping @MartinNowak @CyberShadow @aG0aep6G @JackStouffer as mentioned above we seem to need some meta information per entry, so using a slightly "more complex" file format might be better, e.g. YAML.
In the original discussion it was rejected due to unneeded complexity, but (1) we need a flexibly and simple file format and (2) this PR is stalling anyways, so if no one shouts __within the next week __ I will change the PR to use YAML as file format for the changelog files because imho it's more important to have this process in place than the actual format.

@CyberShadow
Copy link
Member

That sounds like a grossly overcomplicated design.

Why not just put each entry in its own file and use the filenames for sorting?

@wilzbach
Copy link
Contributor Author

wilzbach commented Dec 9, 2016

Why not just put each entry in its own file and use the filenames for sorting?

That would be too simple ;-)
As mentioned above Martin wants at least sorting based on priority and splitting in different categories (e.g. language changes and compiler changes).
Hence I think directly storing the meta information in the file is a lot easier to understand than such a proposed naming scheme:

P1_20161231235959_slug-name.dd scheme. With P1-P5 matching Bugzilla's highest to least priority system.

Probably it would be even P1_language_20161231235959_slug-name.dd :O

@wilzbach
Copy link
Contributor Author

wilzbach commented Dec 9, 2016

Why not just put each entry in its own file and use the filenames for sorting?

(just for the record: that's exactly what at the moment this PR does)

@CyberShadow
Copy link
Member

sorting based on priority and splitting in different categories

But not chronological? Then why the timestamp in the filename? And I don't see a timestamp in your proposed YAML format.

@CyberShadow
Copy link
Member

Probably it would be even P1_language_20161231235959_slug-name.dd :O

IMHO that's preferable to using a serialization language (of which we need a small % of features anyway). Less code is better.

But I defer the decision completely to Martin.

@MartinNowak
Copy link
Member

Let's adopt a color/font-style scheme for the changelog priority instead.
Something for more/less important stuff, and red for deprecations/breakages.
I'll go over the giant 2.072.0 changelog to prioritze stuff and standardize this.

Sorry, didn't got to that, and didn't clearly state the simpler idea.
If we just use sth. like $(BREAKING_CHANGE my title), $(DEPRECATION my title), $(FEATURE my
title), $(BUGFIX my title) for the titles, that should be good enough for grouping and highlighting.
We can infer components from the repo it's in, the distinction between language and compiler changes is vague at least, and could be nicely solved by moving changelogs for language changes to the dlang.org repo (where the specs live).

As mentioned above Martin wants at least sorting based on priority and splitting in different categories (e.g. language changes and compiler changes).

Nothing people ever say is set in stone, unless they say so. Sometimes it's hard to tell from a written messages whether sth. is just an idea or a well thought through concept, which is a mistake of the writter trying to save time. If things seem unclear, at best get in direct contact to people for clarifications.

@wilzbach
Copy link
Contributor Author

Sorry, didn't got to that, and didn't clearly state the simpler idea.
If we just use sth. like $(BREAKING_CHANGE my title), $(DEPRECATION my title), $(FEATURE my
title), $(BUGFIX my title) for the titles, that should be good enough for grouping and highlighting.

Okay, then this means we stick with the plain "github commit message" format :)
After all if we realize some troubles, we can still change it & we will only realize potential problems, if we are working with the single file changelog files.

So is there anything else blocking this PR?

We can infer components from the repo it's in, the distinction between language and compiler changes is vague at least, and could be nicely solved by moving changelogs for language changes to the dlang.org repo (where the specs live).

I like this idea! The changelog builder didn't check dlang.org yet, so I added that (and tools + installer).

@wilzbach
Copy link
Contributor Author

Changelog directory PRs for the repos:

DMD: dlang/dmd#6325
Druntime: dlang/druntime#1716
Phobos: dlang/phobos#4228
Dlang.org: dlang/dlang.org#1524

@PetarKirov
Copy link
Member

Ping @wilzbach @MartinNowak, what's blocking this PR?

@wilzbach
Copy link
Contributor Author

Ping @wilzbach @MartinNowak, what's blocking this PR?

Ehm not much - I guess @MartinNowak needs to flip the switch for the initial version?

@MartinNowak
Copy link
Member

Will do a final review (and merge) later today before branching and building 2.073.0-b0.

@MartinNowak
Copy link
Member

See wilzbach#1

Copy link
Member

@MartinNowak MartinNowak left a comment

Choose a reason for hiding this comment

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

Will merge the followup myself.

@MartinNowak MartinNowak merged commit 3064d99 into dlang:master Jan 6, 2017
MartinNowak added a commit that referenced this pull request Jan 6, 2017
@MartinNowak
Copy link
Member

MartinNowak commented Jan 6, 2017

see #211

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.

5 participants