Skip to content

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Jan 7, 2017

@CyberShadow - do you need anything more than this target to display a preview of the upcoming changelog on your DAutoTest?

CC @MartinNowak

@wilzbach wilzbach force-pushed the add-pending-changelog-target branch from fca476d to 1439372 Compare January 7, 2017 15:11
posix.mak Outdated
pending_changelog:
@echo "This command will be available soon."
pending_changelog_impl: ${STABLE_DMD}
$(STABLE_RDMD) $(TOOLS_DIR)/changed.d -o changelog/pending.dd "v${LATEST}..upstream/stable"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI: if you only want to display the manual changes and hide the changes from resolved Bugzilla issues, we can just remove the "v${LATEST}..upstream/stable" part.
Querying Bugzilla takes about 10-15s.

@wilzbach
Copy link
Contributor Author

Ping @CyberShadow - is there anything else I can do from my side?
You basically just need to merge this PR and add the Makefile target to your awesome DAutoTest

@dnadlinger
Copy link
Contributor

dnadlinger commented Feb 16, 2017

Leaving it up to @CyberShadow to merge, then. (i.e. to review for compatibility with DAutoTest, although I can't see why it shouldn't work)

@CyberShadow
Copy link
Member

CyberShadow commented Feb 16, 2017

Any reason not to include it into the default target?

I.e., let's say I make DAutoTest invoke make to build this target too. Now, should this also be built by Digger when building and deploying the live website? If not, what's the argument for the discrepancy of what gets tested vs. what gets released? If yes, why not include it in the default target?

@CyberShadow
Copy link
Member

CyberShadow commented Feb 16, 2017

BTW, I've mentioned this before, but would be really cool if it would generate pages for the next versions using the current pending changelog, i.e. for 2.074.0 from master and for 2.073.2 from stable.

@wilzbach wilzbach force-pushed the add-pending-changelog-target branch 6 times, most recently from 262bfbf to d469203 Compare February 18, 2017 00:22
@wilzbach
Copy link
Contributor Author

Any reason not to include it into the default target?

Okay I managed to hack this into the default target. The biggest problem was that the changed.d script requires the remote upstream to be set for all repos and it requires the repos tools and installer, but there's nothing that a Makefile can't solve ;-)
Btw I am impressed that it passes and renders correctly, because on my machine it was failing due to dlang/tools#217.

BTW, I've mentioned this before, but would be really cool if it would generate pages for the next versions using the current pending changelog, i.e. for 2.074.0 from master and for 2.073.2 from stable.

I am not sure whether I understand, but I changed the Makefile to write the generated changelog file to changelog/$(cat ../dmd/VERSION).dd which is 2.074.0 for master, but only 2.073.1 for stable.

@wilzbach
Copy link
Contributor Author

@CyberShadow: I am not sure how you want to handle the resulting changelog file, but I guess it's hard without special casing.

image

@CyberShadow
Copy link
Member

CyberShadow commented Feb 18, 2017

The biggest problem was that the changed.d script requires the remote upstream to be set for all repos and it requires the repos tools and installer, but there's nothing that a Makefile can't solve ;-)

I think that should be solved in changed. It should not make assumptions like that.

Note that you can fetch from a URL directly without adding it as a remote. If adding a remote is required, changed should do that and namespace it properly (i.e. e.g. changed_upstream).

I am not sure whether I understand, but I changed the Makefile to write the generated changelog file to changelog/$(cat ../dmd/VERSION).dd which is 2.074.0 for master, but only 2.073.1 for stable.

That might not be a good idea right now, since it doesn't look like updating the DMD VERSION is part of the release process - so it looks like unless it's updated properly, this might actually overwrite or conflict with a released version's changelog? I'd suggest using some simple arithmetics to calculate the next versions using the dlang repo's VERSION file.

Also, this would be a cherry on top but the download links are broken right now, whereas ideally it would link to the latest nightlies. The idea is to allow people to see what's coming in the next official release, and easily test it out if they want to. Same with the stable branch but for upcoming regression fixes.

Anyway, if you don't want to commit to all of that now, we can revert to the simpler version for the moment.

@CyberShadow: I am not sure how you want to handle the resulting changelog file, but I guess it's hard without special casing.

That looks good to me, what's the problem?

@wilzbach wilzbach force-pushed the add-pending-changelog-target branch from d469203 to 8b033ce Compare February 18, 2017 01:55
@wilzbach
Copy link
Contributor Author

wilzbach commented Feb 18, 2017

I think that should be solved in changed. It should not make assumptions like that.
Note that you can fetch from a URL directly without adding it as a remote. If adding a remote is required, changed should do that and namespace it properly (i.e. e.g. changed_upstream).

changed.d parses the git log command.

I am not sure how @MartinNowak depends on this script. I added this to the tools PR as an example (it's just three lines).

Should I keep the Makefile initialization of tools and installer or do you prefer to do this in another way?

That might not be a good idea right now, since it doesn't look like updating the DMD VERSION is part of the release process - so it looks like unless it's updated properly, this might actually overwrite or conflict with a released version's changelog? I'd suggest using some simple arithmetics to calculate the next versions using the dlang repo's VERSION file.

"Simple arithmetics" in a Makefile..
My workaround is to use an ugly bash expression.

Anyway, if you don't want to commit to all of that now, we can revert to the simpler version for the moment.

Well, I am running a bit out of time for now....

That looks good to me, what's the problem?

That as far as I understand it will always be shown as "385 additions" in the status line?

@CyberShadow
Copy link
Member

Should I keep the Makefile initialization of tools and installer or do you prefer to do this in another way?

That looks good, but would be nice to drop the adding of the remotes. They will not be added if the repository already exists, and changed should add them by itself if it can't avoid it.

My workaround is to use an ugly bash expression.

Yep, that's the ticket.

That as far as I understand it will always be shown as "385 additions" in the status line?

No, only the first time the file is created.

@andralex
Copy link
Member

Cool, I see @wilzbach and @CyberShadow have this going. Thanks!

@wilzbach
Copy link
Contributor Author

Cool, I see @wilzbach and @CyberShadow have this going. Thanks!

Well unfortunately this is a bit blocked on dlang/tools#217 (cleaning up the way changed.d fetches the git log which it uses to parse the list of all fixed bugs from)

@wilzbach wilzbach force-pushed the add-pending-changelog-target branch from 8b033ce to 3001827 Compare February 26, 2017 16:56
@wilzbach wilzbach force-pushed the add-pending-changelog-target branch from 3001827 to 2bfe60b Compare February 26, 2017 17:20
@wilzbach
Copy link
Contributor Author

@CyberShadow I think with dlang/tools#217 merged we can finally enable this? :)

@CyberShadow CyberShadow merged commit f092343 into dlang:master Feb 27, 2017
@CyberShadow
Copy link
Member

My wishlist of possible future improvements:

  • Add a notice to the page to make it obvious it's a pre-release
  • Generate a changelog for the stable branch as well
  • Make the download link go to the nightlies
  • Make it generate the release date according to our release schedule
    • Changed "Released" to "To be released"
  • Link to it from the sidebar

First one is most important to avoid confusion. just in case someone stumbles upon that page somehow. I've been demanding enough for this series of changes, though.

@wilzbach wilzbach deleted the add-pending-changelog-target branch February 27, 2017 05:50
@wilzbach
Copy link
Contributor Author

wilzbach commented Feb 27, 2017

Make the download link go to the nightlies
Add a notice to the page to make it obvious it's a pre-release
Changed "Released" to "To be released"

-> #1586

Link to it from the sidebar

Hmm, then it would be easily visible to the public. Do we want this?
We could mark it as "to be released" though.
In any case, there's the update_nav.sh which could be invoked:

-> #1585

Generate a changelog for the stable branch as well

Well I am not sure on the best path here as AFAIK currently @MartinNowak commits the changelog manually. Do you have sth. specific in mind?

@CyberShadow
Copy link
Member

Hmm, then it would be easily visible to the public. Do we want this?

Yes, pertaining that the other issues are resolved.

Well I am not sure on the best path here as AFAIK currently @MartinNowak commits the changelog manually. Do you have sth. specific in mind?

Sorry, but can we not use exactly the same method we use for the master branch but for the stable branch? And use the same method to generate the file name, except increment the minor version number instead of the major one.

@MartinNowak
Copy link
Member

Just name the changlog _pre.d will build a pre-release changlog.


pending_changelog:
@echo "This command will be available soon."
changelog/${NEXT_VERSION}.dd: ${STABLE_DMD} ../tools ../installer
Copy link
Member

Choose a reason for hiding this comment

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

changelog/${NEXT_VERSION}_pre.dd

pending_changelog:
@echo "This command will be available soon."
changelog/${NEXT_VERSION}.dd: ${STABLE_DMD} ../tools ../installer
$(STABLE_RDMD) $(TOOLS_DIR)/changed.d "v${LATEST}..upstream/stable" -o changelog/${NEXT_VERSION}.dd \
Copy link
Member

Choose a reason for hiding this comment

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

Always using stable seems incorrect when the purpose of this target is trying to build the changelog for dmd/druntime/phobos PRs.

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 was already changed, as we realized that showing the bug list from Bugzilla will lead to unrelated diffs on DAutoTest: #1588

@MartinNowak
Copy link
Member

This breaks the build script b/c it builds the pending changelog by default http://nightlies.dlang.org/dmd-master-2017-03-01/build.html.
Should we patch that away or make it configurable?
Does that actually make sense for nightlies (who reads offline nightly docs anyhow?), then we'd need to adapt the changed tool to not rely on git -C path.
See http://nightlies.dlang.org/dmd-master-2017-03-01/build.html.

@wilzbach
Copy link
Contributor Author

wilzbach commented Mar 1, 2017

Should we patch that away or make it configurable?

How about sth. like BUILD_CHANGELOG?

Does that actually make sense for nightlies (who reads offline nightly docs anyhow?)

Nope doesn't make sense, you are absolutely correct ;-)

@CyberShadow
Copy link
Member

Does that actually make sense for nightlies (who reads offline nightly docs anyhow?)

Does it matter?

Seems the problem is that the machine building the nightlies has an old version of git? Could that be solved instead?

-C seems like a fairly recent option, but it can be substituted with --work-tree and --git-dir.
https://github.com/CyberShadow/D-dot-git/blob/9ce30c21bd56c94494403886c1d9ff76735f68ab/repo.d#L26

@MartinNowak
Copy link
Member

Yes, it matters, we don't want to build 2.075 changelogs for 2.074 releases, so we need some option to skip that eventually.
For preview builds we'd rather want the changelog, though generation isn't yet fully automated (some small issues w/ changed.d and update_nav.sh) and the changed tools doesn't work w/ the shallow clones from the builds script.
So for now, I'd rather just disable this in the builds script.

@CyberShadow
Copy link
Member

Yes, it matters, we don't want to build 2.075 changelogs for 2.074 releases, so we need some option to skip that eventually.

Ah, I thought it was already skipped for releases?

@wilzbach
Copy link
Contributor Author

wilzbach commented Mar 2, 2017

Ah, I thought it was already skipped for releases?

It's a pending PR: #1586

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