Fix a few bugs with the changelog creation script #13020
Conversation
bin/create-release
Outdated
| # 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']) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
bin/dotd
Outdated
| @@ -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\\\"\"" | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
oooh, I love the idea of a helper
There was a problem hiding this comment.
run_on helper added; unfortunately had to do a little escaping to deal with bash/ssh's pickiness, Let me know what you think
bin/dotd
Outdated
| @@ -167,7 +173,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 \\\"cd production; ./bin/create-release\\\"\"" | |||
| run_on("production-daemon", "cd production; ./bin/create-release") | |||
bin/dotd
Outdated
| def run_on(server_name, command) | ||
| # escape any double quotes in the command string | ||
| command = %Q["#{command}"] | ||
| system "ssh -t gateway.code.org ssh -t #{server_name} \"#{command}\"" |
There was a problem hiding this comment.
Does ruby implicitly handle escaping pre-escaped quotes when it interpolates here? If not, I worry you'll get
def run_on(server_name, command) # command='my "command"'
command = %Q["#{command}"] # command='my \"command\"'
system "ssh -t gateway.code.org ssh -t #{server_name} \"#{command}\""
# 'ssh -t gateway.code.org ssh -t server \"my \"command\"\"'There was a problem hiding this comment.
Hmm. I thought it did, because the script worked despite the content push step having nested quotes, but upon closer inspection it looks like what you have above is exactly what we'd get.
In other words, I'm not actually sure what's going on here. I'll investigate more.
There was a problem hiding this comment.
Ahh, I forgot that we're going up through two levels of server, so we actually need that many levels of escaping; as it turns out, we weren't actually wrapping user_name in quotes in our original call to content-push; giving it a name with whitespace would cause it to blow up.
No longer!
|
Circle failure is unrelated |
EDITORenvironment variable, becausenil.to_sreturns a truthy empty stringcreate-releasescript from the home directory, but it needs to be run from within the github repo in order forgit logto work