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

Make blog link unique #242

Closed

Conversation

5archoufa
Copy link

Include date in URL

Copy link

sonarcloud bot commented Apr 23, 2024

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Member

@jdrueckert jdrueckert left a comment

Choose a reason for hiding this comment

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

From looking at the code changes (and ignoring the ones duplicating #239), this looks reasonable. I'll need to test it, though, before merging. I'll try to find time to do that this weekend, not sure I will though, so apologies as it might take another week before I get to merge this.

Meanwhile please remove the unused var declaration in Blog.jsx and the changes duplicating your other PR.

Comment on lines +23 to +25

const codeBlockRegex = /```([\s\S]*?)```/g;

Copy link
Member

Choose a reason for hiding this comment

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

as you're not referencing codeBlockRegex anywhere, I assume this can be removed

@jdrueckert
Copy link
Member

jdrueckert commented May 8, 2024

@5archoufa I locally tested this and noticed two issues:

  1. I don't think we need the entire date and timestamp in the blog link. Currently it's unlikely that we even write two blog posts in a month... Using the date should be enough even if the website picks up in blogs written again in the future.
  2. The blog preview cards on the front page don't properly navigate to the correct post when clicking on "Read more". Seems like they still use the "old" (not unique) links.

Also, prettier complains about some formatting issues. You can check the warnings out locally using yarn run format-check:js and run prettier on the affected files using yarn prettier <path/to/file> --write to fix the formatting issues.

@Rukki13
Copy link
Contributor

Rukki13 commented May 11, 2024

I've made a pull request here: #244

@jdrueckert
Copy link
Member

I've made a pull request here: #244

Okay, then I'll close this one. But, just fyi, you don't need to open a new PR whenever you add changes. You can simply push to the same branch again and the PR should update.

@jdrueckert jdrueckert closed this May 15, 2024
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