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 release script #216

Merged
merged 5 commits into from Aug 16, 2020
Merged

Fix release script #216

merged 5 commits into from Aug 16, 2020

Conversation

@say4n
Copy link
Member

@say4n say4n commented Aug 10, 2020

Fix release script to use CLI arguments instead of the hardcoded ones.

@say4n
Copy link
Member Author

@say4n say4n commented Aug 10, 2020

I will update HISTORY.md once #214 is merged.

@say4n
Copy link
Member Author

@say4n say4n commented Aug 11, 2020

Alright, now I will rebase on top of the current upstream master and then update HISTORY.md accordingly.

say4n added 2 commits Aug 10, 2020
@say4n say4n force-pushed the say4n:fix-release-script branch from 3d27d93 to 203de9f Aug 11, 2020
@say4n say4n changed the title Fix release script. Fix release script Aug 11, 2020
Copy link
Member

@rcurtin rcurtin left a comment

Thanks for looking into this! I think I might have found a bug in the awk program---let me know what you think. 👍

scripts/ensmallen-release.sh Outdated Show resolved Hide resolved
Co-authored-by: Ryan Curtin <ryan@ratml.org>
Copy link
Member

@rcurtin rcurtin left a comment

Thanks, looks good to me! 👍

@zoq
zoq approved these changes Aug 12, 2020
Copy link
Member

@zoq zoq left a comment

Looks good to me.

@@ -23,7 +23,8 @@ if [ "$#" -gt 5 ]; then
fi

# Make sure that the branch is clean.
lines=`git diff | wc -l`;
# Truncate leading whitespaces since wc -l on MacOS adds an extra \t.

This comment has been minimized.

@zoq

zoq Aug 12, 2020
Member

I think it's macOS but don't think people will be confused.

This comment has been minimized.

@coatless

coatless Aug 16, 2020
Contributor

It's macOS... No one will be confused. just don't name it OS X 😉

This comment has been minimized.

@rcurtin

rcurtin Aug 16, 2020
Member

Is it a roman numeral or not? Do I pronounce it X or 10? Question for the ages 😄

@mlpack-bot mlpack-bot bot removed the s: needs review label Aug 12, 2020
@birm
birm approved these changes Aug 16, 2020
Copy link
Member

@birm birm left a comment

Looks promising!

@rcurtin
Copy link
Member

@rcurtin rcurtin commented Aug 16, 2020

Thanks @say4n!

@rcurtin rcurtin merged commit 0445b10 into mlpack:master Aug 16, 2020
4 checks passed
4 checks passed
Memory Checks Build finished.
Details
Static Code Analysis Checks Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@say4n say4n deleted the say4n:fix-release-script branch Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.