Skip to content

Deprecate --nightly in favor of --release-type#251

Closed
wilzbach wants to merge 3 commits intodlang:masterfrom
wilzbach:release-type
Closed

Deprecate --nightly in favor of --release-type#251
wilzbach wants to merge 3 commits intodlang:masterfrom
wilzbach:release-type

Conversation

@wilzbach
Copy link
Copy Markdown
Contributor

It will forward the logic to a DDoc template.

@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

if (!revRange.empty)
{
if (useNightlyTemplate)
w.put("$(BR)$(BIG $(RELATIVE_LINK2 bugfix-list, List of all upcoming bug fixes and enhancements.))\n\n");
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I doubt that anyone cared about this and this simplifies the generation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand how this really simplifies generation, and I think saying "list of things in $VERSION" when $VERSION hasn't been released yet is a little bit worse?

@CyberShadow
Copy link
Copy Markdown
Member

What's this for, _pre / _next ?

@wilzbach
Copy link
Copy Markdown
Contributor Author

What's this for, _pre / _next ?

Yep the generated changelog - see dlang/dlang.org#1821 for details.

Copy link
Copy Markdown
Member

@CyberShadow CyberShadow left a comment

Choose a reason for hiding this comment

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

I don't understand this change. Will we have so many release types that it makes sense to make it an arbitrarily specifiable unvalidated parameter and even go as far as deprecate the old way of specifying the release type?

Sorry it took me a while to look at this properly as I was rather confused about a lot of things here.

"version", &nextVersionString,
"nightly", &useNightlyTemplate,
"nightly", &useNightlyTemplate, // deprecated, use --version=nightly
"release-type", &releaseType, // e.g. beta or nightly (stable by default)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not:

"nightly", { releaseType = "NIGHTLY_"; },
"beta", { releaseType = "BETA_"; },

else
w.formattedWrite("$(VERSION %s, =================================================,\n\n", nextVersionDate);
import std.uni : asUpperCase;
w.formattedWrite("$(%sVERSION %s, =================================================,\n\n", text(releaseType.asUpperCase, "_"), nextVersionDate);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why append the underscore to the releaseType separately, instead of putting the underscore in the format string? Did you mean to make the releaseType including underscore optional?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why tie the name of a command-line switch to the name of a DDoc template?

if (!revRange.empty)
{
if (useNightlyTemplate)
w.put("$(BR)$(BIG $(RELATIVE_LINK2 bugfix-list, List of all upcoming bug fixes and enhancements.))\n\n");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand how this really simplifies generation, and I think saying "list of things in $VERSION" when $VERSION hasn't been released yet is a little bit worse?

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

if (revRange.length)
if (revRange.empty)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Err, did you mean to negate this?

@wilzbach
Copy link
Copy Markdown
Contributor Author

I don't understand this change. Will we have so many release types that it makes sense to make it an arbitrarily specifiable unvalidated parameter

We can also add a beta flag, of course, but it just felt wrong to arbitrarily limit the input. FYI Martin has talked about planning to change the release process to match semantic versioning soon.

and even go as far as deprecate the old way of specifying the release type?

This script is solely used for dlang.org (it should imho actually be part of that repository), so the deprecation was just because it wasn't hard to maintain the current behavior and to avoid breaking dlang.org before the other PR has been merged. I would have removed it afterwards to keep things clean and without unnecessary baggage.

Sorry it took me a while to look at this properly as I was rather confused about a lot of things here

Don't worry - it wasn't a full day!!
Besides I am busy with other stuff over the next days, so it might take me a bit to finish this one up.

@CyberShadow
Copy link
Copy Markdown
Member

This script is solely used for dlang.org

Isn't the script also used by the release manager (Martin) to build the static changelog page for a release?

@wilzbach
Copy link
Copy Markdown
Contributor Author

Isn't the script also used by the release manager (Martin) to build the static changelog page for a release?

Yes, but I think/hope that he uses the changelog/${NEXT_VERSION}_pre.dd Makefile target at dlang.org, which reminds me that now that the pending_changelog target creates a _pending file, we should maybe add a pre_changelog target for him.

@MartinNowak
Copy link
Copy Markdown
Member

MartinNowak commented Jul 16, 2017

I'd like to stick with the previous mechanism, remove any notion of different release types from the changed tool, and use different macros in dlang.org's makefile for _pre.dd and _xyz.dd sources.
See CHANGELOG_PRE_DDOC.

@wilzbach
Copy link
Copy Markdown
Contributor Author

Superseded by #252

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.

4 participants