Skip to content
This repository has been archived by the owner on Apr 1, 2022. It is now read-only.

Parses entry in pod and dependency section correctly when it is enclosed with quotation #337

Merged
merged 3 commits into from
Aug 20, 2021

Conversation

meghfossa
Copy link
Contributor

@meghfossa meghfossa commented Aug 19, 2021

Overview

It correctly parses when Podlock file has quotations in entry for dependency and pod section.

Acceptance criteria

  • Parse Entry in Pod correctly when enclosed with quotation
  • Parse Entry in Dependency correctly when enclosed with quotation

Testing plan

  • I've included unit tests to of those effect

For reproducible example,

  • Setup simple pod project
  • Add pod 'GoogleUtilities' in Podfile
  • Execute pod install
  • Analyze Pod project with applied fixes
  • Analyze Pod project without applied fixes

Risks

N/A

References

Works on https://github.com/fossas/team-analysis/issues/716

Checklist

  • I added tests for this PR's change (or confirmed tests are not viable).
  • If this PR introduced a user-visible change, I added documentation into docs/.
  • I updated Changelog.md if this change is externally facing. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
  • I linked this PR to any referenced GitHub issues, if they exist.

@meghfossa meghfossa requested review from a team and elldritch and removed request for a team August 19, 2021 03:34
@meghfossa meghfossa changed the title Parses entry in pod and dependency section correctly when it has enclosed with quotation Parses entry in pod and dependency section correctly when it is enclosed with quotation Aug 19, 2021
Copy link
Contributor

@cnr cnr left a comment

Choose a reason for hiding this comment

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

Now that we know Podfile.lock is valid yaml (see: these lines in cocoapods' lockfile.rb), let's replace the hand-rolled megaparsec parser with the usual Data.Yaml parser

Using the yaml parser should incidentally resolve the quotes issue, too

@meghfossa
Copy link
Contributor Author

Now that we know Podfile.lock is valid yaml (see: these lines in cocoapods' lockfile.rb), let's replace the hand-rolled megaparsec parser with the usual Data.Yaml parser

Using the yaml parser should incidentally resolve the quotes issue, too

I agree, and I don't mind addressing this - but this would increase scope of this PR. I don't mind adding tech-debt ticket and refactoring this in secondary PR right after. Trying to get the changes in this pr may introduce additional time delay on rollout.

@meghfossa
Copy link
Contributor Author

@cnr per request, I've added PR to #340 to use YAML parsing. But I would like to move ahead with this PR first given the time sensitive nature of this. Would you be able to remove request for change, if the sibling PR satisfies your concerns?

@meghfossa meghfossa requested review from a team and removed request for elldritch and a team August 20, 2021 14:07
Copy link
Contributor

@cnr cnr left a comment

Choose a reason for hiding this comment

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

LGTM

@meghfossa meghfossa force-pushed the fix/podlock-yaml-quotes branch from f93ed61 to 9837147 Compare August 20, 2021 16:27
@meghfossa meghfossa force-pushed the fix/podlock-yaml-quotes branch from 9837147 to 8ad7433 Compare August 20, 2021 16:30
@meghfossa meghfossa merged commit a15e018 into master Aug 20, 2021
@meghfossa meghfossa deleted the fix/podlock-yaml-quotes branch August 20, 2021 16:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants