Skip to content

Conversation

@hiranya911
Copy link
Contributor

No description provided.

Copy link
Contributor

@avishalom avishalom left a comment

Choose a reason for hiding this comment

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

LGTM, if you like I suggested a change to make the sed -ing more robust

# UPDATE CHANGELOG.md #
#########################
echo "[INFO] Updating version number in CHANGELOG.md to ${VERSION_WITHOUT_RC}..."
sed -i '' -e s/"# Unreleased"/"# v${VERSION_WITHOUT_RC}"/ CHANGELOG.md
Copy link
Contributor

@avishalom avishalom Jan 23, 2018

Choose a reason for hiding this comment

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

This seems a bit fragile, What if "# Unreleased" appears anywhere else.
Also, it seems that the current version doesn't have a blank line in line number 2 .
What if it was deleted in the process.

I suggest,

  • only sed-ing the first line,
  • replace "# Unreleased" or ignore it and append if it isn't there
  • append to the end of the first line so that anything non congruent stands out
    s.t. like

sed -i '' -E '1 s/(# Unreleased|^)/# Unreleased'$'\\n\\n-\\n'"# v${VERSION_WITHOUT_RC}/" CHANGELOG.md

instead of lines 95-97

Copy link
Contributor Author

@hiranya911 hiranya911 Jan 23, 2018

Choose a reason for hiding this comment

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

Updated the sed command to only replace the first occurrence. Points 2 and 3 are hard to implement in a way that is compatible with both Linux and Mac. So I'm keeping the simple cat to file, and move solution for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

My only worry is that if someone accidentally changes that first line, the version will not be added.
How about replacing the first "# Unreleased" with nothing ("") and adding the v$VERSION_WITHOUT_RC in the echo in the next line ?

@avishalom avishalom assigned hiranya911 and unassigned avishalom Jan 23, 2018
@hiranya911 hiranya911 merged commit 795ff13 into master Jan 23, 2018
@hiranya911 hiranya911 deleted the hkj-update-log branch January 23, 2018 21:49
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.

2 participants