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

Implemented UK_ENGLISH Locale #106

Merged
merged 9 commits into from
Oct 2, 2020
Merged

Conversation

LilySu
Copy link
Contributor

@LilySu LilySu commented Sep 19, 2020

Implemented UK_ENGLISH Implemented Search

Description

Implemented UK_ENGLISH as an option for 'locale' in the same way USA_ENGLISH and CANADA_ENGLISH are implemented, except with 'co.uk' for Indeed and Monster.

This involved taking out the state or province in the URL for get_search_url in indeed.py and monster.py in subclass: IndeedScraperUKEng and MonsterScraperUKEng.

Context of change

Please add options that are relevant and mark any boxes that apply.

  • Software (software that runs on the PC)
  • Library (library that runs on the PC)
  • Tool (tool that assists coding development)
  • Other

Type of change

Please mark any boxes that apply.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Ran UK_ENGLISH in my_settings.yaml with cities in the UK such as London and Cambridge for Indeed and Monster.

Checklist:

Please mark any boxes that have been completed.

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • Any dependent changes have been merged and published in downstream modules.

@LilySu LilySu changed the title Implement UK_ENGLISH and city: remote option for Indeed and Monster Implemented UK_ENGLISH and city: remote option for Indeed and Monster Sep 19, 2020
Copy link
Owner

@PaulMcInnis PaulMcInnis left a comment

Choose a reason for hiding this comment

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

Glad to see UK English support!

Suggesting some small changes.

demo/settings.yaml Outdated Show resolved Hide resolved
jobfunnel/backend/scrapers/glassdoor.py Outdated Show resolved Hide resolved
jobfunnel/backend/scrapers/glassdoor.py Outdated Show resolved Hide resolved
jobfunnel/backend/scrapers/indeed.py Outdated Show resolved Hide resolved
jobfunnel/backend/scrapers/indeed.py Outdated Show resolved Hide resolved
jobfunnel/backend/scrapers/indeed.py Outdated Show resolved Hide resolved
jobfunnel/backend/scrapers/monster.py Outdated Show resolved Hide resolved
jobfunnel/backend/scrapers/monster.py Outdated Show resolved Hide resolved
@PaulMcInnis PaulMcInnis linked an issue Sep 19, 2020 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Sep 19, 2020

Codecov Report

Merging #106 into master will increase coverage by 0.22%.
The diff coverage is 54.16%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
+ Coverage   35.68%   35.90%   +0.22%     
==========================================
  Files          22       22              
  Lines        1390     1412      +22     
==========================================
+ Hits          496      507      +11     
- Misses        894      905      +11     
Impacted Files Coverage Δ
jobfunnel/backend/scrapers/registry.py 100.00% <ø> (ø)
jobfunnel/resources/defaults.py 100.00% <ø> (ø)
jobfunnel/backend/scrapers/indeed.py 28.57% <28.57%> (ø)
jobfunnel/backend/scrapers/monster.py 26.97% <44.44%> (+0.58%) ⬆️
jobfunnel/backend/scrapers/base.py 38.56% <75.00%> (+0.97%) ⬆️
jobfunnel/backend/scrapers/glassdoor.py 30.93% <100.00%> (+1.00%) ⬆️
jobfunnel/resources/enums.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d0297e...eb20bc5. Read the comment docs.

@LilySu LilySu marked this pull request as draft September 26, 2020 03:48
@PaulMcInnis PaulMcInnis marked this pull request as ready for review September 27, 2020 23:33
Copy link
Owner

@PaulMcInnis PaulMcInnis left a comment

Choose a reason for hiding this comment

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

Almost there, Lets share the radius calculation as suggested, and remove the YAML discussing the previously-implemented 'remote' for city option. 👍

demo/settings.yaml Outdated Show resolved Hide resolved
jobfunnel/backend/scrapers/glassdoor.py Outdated Show resolved Hide resolved
Copy link
Owner

@PaulMcInnis PaulMcInnis left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Collaborator

@markkvdb markkvdb left a comment

Choose a reason for hiding this comment

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

I tested the UK implementation on a Mac and it works as expected. Great job!

@PaulMcInnis PaulMcInnis added this to the 3.0.1 milestone Sep 30, 2020
@PaulMcInnis PaulMcInnis linked an issue Sep 30, 2020 that may be closed by this pull request
@PaulMcInnis PaulMcInnis changed the title Implemented UK_ENGLISH and city: remote option for Indeed and Monster Implemented UK_ENGLISH Locale Sep 30, 2020
@PaulMcInnis PaulMcInnis merged commit 27fb84c into PaulMcInnis:master Oct 2, 2020
@markkvdb markkvdb mentioned this pull request Oct 2, 2020
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement European locales
4 participants