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 Strange quotation marks in lockfile.rb #381

Merged
merged 1 commit into from
Jun 8, 2017

Conversation

dacaiguoguogmail
Copy link
Contributor

@dacaiguoguogmail dacaiguoguogmail commented Jun 4, 2017

Strange quotation marks in 'SPEC CHECKSUMS' section

SPEC CHECKSUMS:
  SDWebImage: '098e97e6176540799c27e804c96653ee0833d13c'

@orta
Copy link
Member

orta commented Jun 5, 2017

I'm for this TBH, the quotes thing is an odd side-effect of different versions of the yaml writer between ruby versions that shouldn't affect reading I imagine.

@dacaiguoguogmail
Copy link
Contributor Author

But the development team is not unified version of the situation is easy to appear, resulting in git log quotation marks appear and disappear, appear again, it is unnecessary and very troublesome.
Can you make yaml writer compatible with different ruby versions? Or given yaml writer compatible ruby version?

@orta
Copy link
Member

orta commented Jun 5, 2017

Can you make yaml writer compatible with different ruby versions? Or given yaml writer compatible ruby version?

IMO - this is what your PR does 👍

@dacaiguoguogmail
Copy link
Contributor Author

So should my PR be accepted?

@orta
Copy link
Member

orta commented Jun 5, 2017

Yes, that is what I said I was in favour of, and it looks like @dnkoutso is too from the other PR

@dnkoutso
Copy link
Contributor

dnkoutso commented Jun 5, 2017

@dacaiguoguogmail why couldnt we just always do something like:

yaml_string.tr("'", '')

Instead?

Yes I think we can merge this generally.

@dacaiguoguogmail
Copy link
Contributor Author

@dnkoutso good idea, new PR #382

@dnkoutso
Copy link
Contributor

dnkoutso commented Jun 6, 2017

@dacaiguoguogmail there is no need to re-open a new PR for each attempt to land this. It makes it harder to come back to and read any comments or thoughts during landing this.

@dnkoutso
Copy link
Contributor

dnkoutso commented Jun 6, 2017

This looks good to me and harmless. Will wait tomorrow to merge it.

@dnkoutso
Copy link
Contributor

dnkoutso commented Jun 6, 2017

Actually will wait until 1.3.0.beta.1 is out and then move it for beta.2

@dnkoutso
Copy link
Contributor

dnkoutso commented Jun 8, 2017

@dacaiguoguogmail can you please rebase and squash your branch so we can land this?

@dacaiguoguogmail
Copy link
Contributor Author

@dnkoutso rebase has been completed

@dnkoutso
Copy link
Contributor

dnkoutso commented Jun 8, 2017

@dacaiguoguogmail this is still not right. Can you please squash to a single commit?

@dacaiguoguogmail
Copy link
Contributor Author

1.I rebase and squash to a single commit in local branch
2.then I git pull, lead to a merge commit
3.git push
How do I deal with this? @dnkoutso

@dacaiguoguogmail
Copy link
Contributor Author

@dnkoutso, I use git push --force fix it

@dnkoutso dnkoutso merged commit 7542753 into CocoaPods:master Jun 8, 2017
@dacaiguoguogmail dacaiguoguogmail deleted the fix-lockfile branch June 9, 2017 07:35
UnsafePointer pushed a commit to UnsafePointer/Core that referenced this pull request Dec 6, 2017
This should fix an issue introduced by CocoaPods#381 with keys that contain !
UnsafePointer pushed a commit to UnsafePointer/Core that referenced this pull request Dec 6, 2017
This should fix an issue introduced by CocoaPods#381 with keys that contain !
UnsafePointer pushed a commit to UnsafePointer/Core that referenced this pull request Dec 6, 2017
This should fix an issue introduced by CocoaPods#381 with keys that contain !
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.

None yet

4 participants