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 incorrect slugification of company names #397

Merged
merged 2 commits into from
Jun 6, 2024
Merged

Conversation

brianlove
Copy link
Contributor

Ignore slashes and periods when slugifying company names.

Closes #365

@brianlove brianlove requested a review from jmelot June 3, 2024 20:58
@brianlove brianlove self-assigned this Jun 3, 2024
Copy link

github-actions bot commented Jun 3, 2024

No need for rebasing 👍
behind_count is 0
ahead_count is 2

Copy link

github-actions bot commented Jun 3, 2024

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
594 423 71% 0% 🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: b0305e9 by action🐍

Copy link

github-actions bot commented Jun 3, 2024

JavaScript Coverage

Summary

Lines Statements Branches Functions
Coverage: 67%
67.67% (404/597) 55.18% (213/386) 68.27% (127/186)
Modified Files • (67%)
File% Stmts% Branch% Funcs% LinesUncovered Line #s
All files67.6755.1868.2767.43 
util83.5152.7284.3786.58 
   index.js83.3355.557092.8547

Copy link
Member

@jmelot jmelot left a comment

Choose a reason for hiding this comment

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

This works! Could you check 2176/Macy's - that one isn't loading for me for some reason (may not be related)

brianlove added 2 commits June 5, 2024 13:50
Ignore slashes and periods when slugifying company names.

Closes #365
Standardize the slugification process across page generation (via
gatsby-config.js) and link generation (src/util/index.js), using the
@sindresorhus/slugify package used internally by Gatsby.  Switch how
names like "Jd.Com" are slugified to be consistent with sindresorhus's
approach.
@brianlove brianlove force-pushed the 365-fix-periods-in-slug branch from 1aaebbc to b0305e9 Compare June 5, 2024 20:57
@brianlove
Copy link
Contributor Author

@jmelot Found and fixed the issue - Gatsby used a different slugification process, so there wasn't agreement between the pages that were actually generated and the links that were generated.

@brianlove brianlove requested a review from jmelot June 5, 2024 20:59
Copy link
Member

@jmelot jmelot left a comment

Choose a reason for hiding this comment

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

👍 thanks for debugging that!

@jmelot jmelot merged commit b304785 into master Jun 6, 2024
5 checks passed
@jmelot jmelot deleted the 365-fix-periods-in-slug branch June 6, 2024 20:15
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.

broken detail view link when company name ends in period
2 participants