-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Docs: Link to rendered GitHub AsciiDoc for release notes #2117
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
base: gh-pages
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ | |
| config = YAML.load_file("hugo.yml") | ||
| config["params"] = {} if config["params"].nil? | ||
| config["params"]["latest_version"] = version | ||
| config["params"]["latest_relnote_url"] = "https://github.com/git/git/raw/HEAD/Documentation/RelNotes/#{version}.adoc" | ||
| config["params"]["latest_relnote_url"] = "https://gitlab.com/git-scm/git/-/blob/v2.52.0/Documentation/RelNotes/#{version}.adoc" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will break for later versions: You are hard-coding v2.52.0 as the revision in which to look for the release notes. Naturally, this revision only contains the release notes leading up to that version, and once, say, v2.52.1 or v2.53.0 come out, the updated link will 404 because the v2.52.0 revision won't magically change to include the release notes for those new versions. You need to either replace |
||
| config["params"]["latest_release_date"] = date.strftime("%Y-%m-%d") | ||
| yaml = YAML.dump(config).gsub(/ *$/, "") | ||
| File.write("hugo.yml", yaml) | ||
| File.write("hugo.yml", yaml) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, too, the hard-coded
v2.52.0revision would come back to bite you, if merged unchanged.The natural fix would be to replace it by
HEAD.But a more fundamental question that should immediately come to your mind when you make virtually the same change in two separate places (the
hrefvalue ininstall-header.htmlas well as thelatest_relnote_urlvalue inupdate-git-version.rb): Why is this URL defined in two separate locations? That's redundant.In this particular instance, the obvious question is: if
update-git-version.rbgoes out of its way to define a global parameterlatest_relnote_urlin addition tolatest_version, why on Earth is thatlatest_relnote_urlparameter not used here?Paying attention to such details and acting on them while you develop patches will automatically improve the quality of your work by ten times, I am sure.