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

Domain, explode url and save favicon #14 [TGER-129] #65

Merged
merged 14 commits into from
Mar 18, 2021

Conversation

devjk1
Copy link
Contributor

@devjk1 devjk1 commented Mar 17, 2021

#14

  • add Helper method ParseUrl
  • add Helper method GetFaviconUrl
  • add Test for ParseUrl
  • add Test for GetFaviconUrl

errors:

  • tests cannot find both Helper methods, src/Helpers

@devjk1 devjk1 changed the title Omnia/feature/14 domain explode url and save favicon Domain, explode url and save favicon #14 [TGER-129] Mar 17, 2021
@pdbreen
Copy link

pdbreen commented Mar 17, 2021

For loading helper functions, combine into single PHP file - say src/Helpers/Helpers.php then update the autoload entry in composer.json with a files entry. You can see an example of this in the tipoff/support:composer.json file.

        "files": [
            "src/Helpers/Helpers.php"
        ]

@devjk1
Copy link
Contributor Author

devjk1 commented Mar 18, 2021

Tests working now, but I'm not out of the woods yet.
Psalm is complaining about vars $name and $tld not being used / referenced .. in ParseUrl.php (I think)
I initialize all vars, change them in the if block, then use all vars in return [ ].

Thanks for the help earlier Patrick

@pdbreen
Copy link

pdbreen commented Mar 18, 2021

Looks like you should eliminate the initialization of $name and $tld. Every code path results in an assignment, making the initialization value the unused value psalm is complaining about.

@drewroberts drewroberts self-requested a review March 18, 2021 16:22
Copy link
Member

@drewroberts drewroberts left a comment

Choose a reason for hiding this comment

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

Thank you! ❄️

@drewroberts drewroberts merged commit fc9aa22 into main Mar 18, 2021
@drewroberts drewroberts deleted the omnia/feature/14_domain_explode_url_and_save_favicon branch March 18, 2021 21:09
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.

3 participants