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

Fix a few bugs with the changelog creation script #13020

Merged
merged 4 commits into from Feb 3, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions bin/create-release
Expand Up @@ -24,8 +24,8 @@ def git_generate_changelog
end

relevant_matches = matches.select do |match|
# ignore merges from staging; those represent DTTs
match['branch'] != 'staging'
# ignore merges from our various server branches
!match.nil? && !['staging', 'test', 'levelbuilder'].include?(match['branch'])
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an interesting question - do we ignore merges "from levelbuilder" because they're routine server-branch merges and so common as to be uninteresting, or do we include them because they represent legitimate content changes that may not happen in every published version?

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't ignore them, IMO. It is often useful to know whether a scoop happened or not. In fact, it would be awesome to include timestamps (this would make it more helpful to non-eng, I'd guess).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I can see that. Particularly because we link to the contents of the scoop itself, it makes it easy to look for particular changes.

end

items = relevant_matches.map do |match|
Expand Down Expand Up @@ -82,7 +82,7 @@ end

def edit(content)
file = Tempfile.new(['create-release', '.md'])
editor = ENV['EDITOR'].to_s || 'vim'
editor = ENV['EDITOR'] || 'vim'
begin
file.write(content)
file.close
Expand Down
2 changes: 1 addition & 1 deletion bin/dotd
Expand Up @@ -167,7 +167,7 @@ should_i "DTP" do

should_i "create a new release of the code-dot-org repository on github" do
wait_for "the production server to fetch the latest changes"
system "ssh -t gateway.code.org ssh -t production-daemon \"production/bin/create-release\""
system "ssh -t gateway.code.org ssh -t \"production-daemon \\\"cd production; ./bin/create-release\\\"\""
Copy link
Contributor

Choose a reason for hiding this comment

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

faint

My eyes glaze over anytime I see nested escaped quotes. Can we mix quote types to make this a little less painful? Or even better, extract a helper run_on('production-daemon', 'cd production; ./bin/create-release') to hide the complexity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oooh, I love the idea of a helper

Copy link
Contributor Author

Choose a reason for hiding this comment

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

run_on helper added; unfortunately had to do a little escaping to deal with bash/ssh's pickiness, Let me know what you think

end

wait_for "DTP to finish"
Expand Down