Skip to content

fixed prevVersion detection on windows #90

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

fixed prevVersion detection on windows #90

wants to merge 1 commit into from

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Feb 24, 2016

No description provided.

@jquerybot
Copy link

Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA).

📝 Please visit http://contribute.jquery.org/CLA/ to sign.

After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know.

If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check.

@staabm
Copy link
Contributor Author

staabm commented Feb 24, 2016

after applying this fix, I also found that contributors.txt is sprinkeled with single quotes. I guess we needs this kind of fix in several spots:

'Brahim Arkni
'Christopher Bauer
'Daniel Orner
'Denisson Padilha
'dorner
'Ehsan Abidi Ashtiani
'Filip Mares
'Jörn Zaefferer
'Julio Spader
'KJ
'Lars Skjelbek
'Lukasz Wozniak
'Markus Staab
'Martti Laine
'Michal Moravec
'netzkind
'rasstislav
'sadj
'Salahuddin Hairai
'Saldanha
'silovski
'The Gitter Badger
'Thomas Gohard
'unknown
'Wojciech Wałek
'Štefan Baebler
Alok Saldanha'
Brahim Arkni'
Christopher Bauer'
Daniel Orner'
Jörn Zaefferer'
Julio Spader'
Markus Staab'
Martti Laine'
sadj'
Salahuddin Hairai'
Thomas Gohard'

@staabm
Copy link
Contributor Author

staabm commented Feb 24, 2016

adjusted the PR with some more fixes. As is I can build jquery-validate on my windows machine

@staabm
Copy link
Contributor Author

staabm commented Feb 24, 2016

see http://stackoverflow.com/questions/24173825/what-does-single-quote-do-in-windows-batch-files/24181667#24181667 for background

Arkni
Arkni reviewed Feb 24, 2016
View reviewed changes
lib/repo.js
@@ -237,14 +237,14 @@ Release.define({
Release.chdir( Release.dir.repo );
console.log( "Committing release artifacts..." );
Release.exec( "git add -f " + paths.join( " " ), "Error adding release artifacts to git." );
Release.exec( "git commit -m '" + Release.newVersion + "'",
Release.exec( "git commit -m \"Release:" + Release.newVersion + "\"",
Copy link

Choose a reason for hiding this comment

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

no need to prefix the commit with Release:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will check that again. I guess I used somehow a old version of commitplease (maybe because of outdated nodejs or npm)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

verified in #88, this is no longer required as the version is (using this PR) properly dected (without the single quotes)

@staabm
Copy link
Contributor Author

staabm commented Feb 25, 2016

just tried to verify the CLA, but the link posted by the BOT above leads to a 404 page.
(I already signed the CLA for my contributions for jquery.js in the past)

staabm added a commit to jquery-validation/jquery-validation that referenced this pull request Feb 25, 2016
@staabm
Build: removed node regarding patching of jquery-release
cceefe8 
will be fixed by jquery/jquery-release#90
@jzaefferer
Copy link
Member

Re CLA: You've signed with [email protected], but your git email is [email protected]. You could sign again with the other email or update your commit.

@staabm
Copy link
Contributor Author

staabm commented Feb 25, 2016

here we go, @jzaefferer thx for the heads up

@jzaefferer
Copy link
Member

Changes look fine to me. Would be useful to update the commit message to explain why this is necessary (link to Stackoverflow?).

@staabm
Copy link
Contributor Author

staabm commented Feb 25, 2016

agree. adjusted per feedback.

@jzaefferer jzaefferer closed this in 4b207e8 Mar 3, 2016
@jzaefferer
Copy link
Member

Thanks!

@staabm staabm deleted the patch-1 branch March 3, 2016 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA: Valid
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@staabm @jquerybot @jzaefferer @Arkni