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

docs: Tweak release docs and improve release build script #7631

Merged
merged 2 commits into from Jul 2, 2018

Conversation

Projects
None yet
2 participants
@tofumatt
Member

tofumatt commented Jun 29, 2018

Fixes #7607

This improves some wording/formatting of the release doc (it should look a lot better especially as output Markdown now), but more importantly stops the build script from removing files in someone's working directory without warning them first.

It does this by doing a --dry-run of git clean -xdf and showing the user the output so they aren't surprised by doing a release and losing their work.

I also prevented the build script from exiting with a non-zero status code, because this really uglies up the script output when an error occurs. I think it's nice to have the error in the script be the last thing the user sees, so this shows the error rather than the npm run error output from exiting with a non-zero status.

Also I added some emoji whimsy because I'm like that.

cc @mtias because he uses these more than anyone else; if these changes make anything worse/less usable for you, let me know and I'll tweak it.

@tofumatt tofumatt requested review from mtias and WordPress/gutenberg-core Jun 29, 2018

@mcsf

mcsf approved these changes Jun 29, 2018

Woot, thanks for working on this! (Left notes)

YELLOW_BOLD='\033[1;33m';
COLOR_RESET='\033[0m';
status () {
echo -e "\n${GREEN_BOLD}$1${COLOR_RESET}\n"

This comment has been minimized.

@mcsf

mcsf Jun 29, 2018

Contributor

Should status use green, or something success-neutral?

This comment has been minimized.

@tofumatt

tofumatt Jun 29, 2018

Member

Yeah maybe not green actually, good point. I was trying to say "this is okay, it's just informational" but green is too happy. I'll save that for the "Done" message.

exit 1
error "ERROR: Cannot build plugin zip with dirty working tree. ☝️
Commit your changes and try again."
exit

This comment has been minimized.

@mcsf

mcsf Jun 29, 2018

Contributor

I'm not saying we shouldn't make this change, but it's yet another thing that makes me dislike npm, that we have to make concessions and break from the Unix pattern.

(Perhaps a more correct fix would be to keep exit 1 and use some process as membrane between this script and npm run, e.g. "package-plugin": "./bin/build-plugin-zip.sh | silent" or "package-plugin": "silence-errors ./bin/build-plugin-zip.sh". Anyway, just a digression, ignore this old soul.)

This comment has been minimized.

@tofumatt

tofumatt Jun 29, 2018

Member

I actually totally agree… I mean, in reality, it's a bash script and shouldn't be run from npm anyway. I should keep the exit codes and just change the docs to say run ./bin/build-plugin-zip.sh.

Would you be okay with that change? We can leave the node script in for people, but encourage straight bash usage 🤷‍♂️

status "Cleaning working directory..."
git clean -xdf
# Do a dry run of the repository reset. Prompting the user for a list of all
# files that will be removed should prevent them from losing important files!

This comment has been minimized.

@mcsf

mcsf Jun 29, 2018

Contributor

Having been bitten by this myself, I say: thank you.

# Do a dry run of the repository reset. Prompting the user for a list of all
# files that will be removed should prevent them from losing important files!
status "Resetting the repository to pristine condition."
echo "Would remove gutenberg.zip"

This comment has been minimized.

@mcsf

mcsf Jun 29, 2018

Contributor

Should it read "Would remove the following:"?

This comment has been minimized.

@tofumatt

tofumatt Jun 29, 2018

Member

It's in there to "blend in" with git clean -xdf's output. So it'll look like:

screen shot 2018-06-29 at 15 51 51

But it's weird out of context; I'll add a code comment.

read answer
if [ "$answer" != "${answer#[Yy]}" ]; then
# Remove any existing zip file.
rm -f gutenberg.zip

This comment has been minimized.

@mcsf

mcsf Jun 29, 2018

Contributor

Wouldn't git clean -xdf take care of this file too?

This comment has been minimized.

@tofumatt

tofumatt Jun 29, 2018

Member

I wondered that and thought yes, but it was in there from before. It doesn't show up in the regular output, I'm guessing because it's in .gitignore so git "knows" about it?

This comment has been minimized.

@tofumatt

tofumatt Jun 29, 2018

Member

Scratch that, it does know about it. Dunno why the deletion was in there before; I'll remove this 🎉

# Remove ignored files to reset repository to pristine condition. Previous
# test ensures that changed files abort the plugin build.
status "Cleaning working directory... 🛀";
git clean -xdf;

This comment has been minimized.

@mcsf

mcsf Jun 29, 2018

Contributor

Not that they break anything, but why the semicolons here?

This comment has been minimized.

@tofumatt

tofumatt Jun 29, 2018

Member

Oops, I think I had a one-line read implementation before and just left them in.

* Check the zip file looks good. Drop it in the #core-editor channel for people to test. Again, make sure if you unzip the file that the version numbers is correct.
* Run `git checkout master` and `git pull`. Make sure your local `master` is up to date; you can confirm this by opening `gutenberg.php` and checking for the version bump you merged previously.
* Run `npm run package-plugin` from the root of project. This packages a zip file with a release build of `gutenberg.zip`.
* Check that the zip file looks good. Drop it in the [`#core-editor` channel](https://wordpress.slack.com/messages/C02QB2JS7) for people to test. Again: make sure if you unzip the file that the version numbers is correct.

This comment has been minimized.

@mcsf

mcsf Jun 29, 2018

Contributor

Typo as "numbers is"

* If you already have a copy, run: `svn up`
* Delete the contents of `trunk` except for the `readme.txt` and `changelog.txt` files (these files don’t exist in the git repo, only in subversion).
* Extract the contents of the zip file to `trunk`.
* Edit `readme.txt`, replacing the changelog for the previous version with current release's changelog.

This comment has been minimized.

@mcsf

mcsf Jun 29, 2018

Contributor

with the current release's

exit
fi
exit 1

This comment has been minimized.

@tofumatt

tofumatt Jun 29, 2018

Member

lol whoops, left this in during testing and forgot to remove it! 😆

tofumatt added some commits Jun 29, 2018

docs: Tweak release docs and improve release build script
Fixes #7607

This improves some wording/formatting of the release doc (it should
look a lot better especially as output Markdown now), but more
importantly stops the build script from removing files in someone's
working directory without warning them first.

It does this by doing a `--dry-run` of `git clean -xdf` and showing
the user the output so they aren't surprised by doing a release
and losing their work.

I also prevented the build script from exiting with a non-zero status
code, because this really uglies up the script output when an error
occurs. I think it's nice to have the error in the script be the last
thing the user sees, so this shows the error rather than the `npm run`
error output from exiting with a non-zero status.

Also I added some emoji whimsy because I'm like that.

@tofumatt tofumatt merged commit 354756a into master Jul 2, 2018

2 checks passed

codecov/project 47.08% (-0.02%) compared to 124647e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tofumatt tofumatt deleted the docs/7607-releasing-deletes-stuff branch Jul 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment