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

feat(rss): add option to remove the trailing slash #6453

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

ematipico
Copy link
Member

Changes

Closes #6427

The PR adds a new option called trailingSlash, in line with the option we have in our config.

This option is useful for having links that are in line with the sitemap.

Testing

Current tests should pass, to keep the existing behaviour. Added a new test case.

Docs

Updated the README.md with the new option.

/cc @withastro/maintainers-docs for feedback!

@ematipico ematipico requested a review from a team as a code owner March 8, 2023 09:57
@changeset-bot
Copy link

changeset-bot bot commented Mar 8, 2023

🦋 Changeset detected

Latest commit: c53bfee

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ematipico ematipico force-pushed the feat/rss-trailing-slash-option branch 2 times, most recently from 9434713 to c5c0f13 Compare March 8, 2023 10:04
@ematipico ematipico force-pushed the feat/rss-trailing-slash-option branch from c5c0f13 to 082d7c2 Compare March 8, 2023 10:45
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Some quick thoughts on this one! This is a great feature. I know people will be looking forward to this!

packages/astro-rss/README.md Outdated Show resolved Hide resolved
packages/astro-rss/README.md Outdated Show resolved Hide resolved
packages/astro-rss/README.md Outdated Show resolved Hide resolved
packages/astro-rss/README.md Outdated Show resolved Hide resolved
.changeset/popular-rules-divide.md Outdated Show resolved Hide resolved
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Implementation looks great! Got a couple suggestions for the docs.

packages/astro-rss/README.md Outdated Show resolved Hide resolved
packages/astro-rss/README.md Outdated Show resolved Hide resolved
packages/astro-rss/README.md Outdated Show resolved Hide resolved
Co-authored-by: Sarah Rainsberger <[email protected]>
@ematipico ematipico requested review from bluwy and sarah11918 March 8, 2023 13:35
'@astrojs/rss': minor
---

Added `trailingSlash` option, to control whether the emitted URLs should have the trailing slash.
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a precedent for verb tense on these? I haven't been picky, but I've now seen the equivalent of:

  • Added
  • Add
  • Adds
    in recent PRs, and it would be nice if we had a standard! Anyone have any thoughts on this?

(Would also make "slashes" plural to agree with plural URLs)

Suggested change
Added `trailingSlash` option, to control whether the emitted URLs should have the trailing slash.
Added `trailingSlash` option to control whether or not the emitted URLs should have trailing slashes.

AND in line 16 below: change to "won't have trailing slashes"

@ematipico ematipico requested a review from sarah11918 March 8, 2023 14:42
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@ematipico ematipico merged commit 2e36204 into main Mar 9, 2023
@ematipico ematipico deleted the feat/rss-trailing-slash-option branch March 9, 2023 07:57
@astrobot-houston astrobot-houston mentioned this pull request Mar 9, 2023
mooreds added a commit to FusionAuth/fusionauth-site that referenced this pull request Aug 16, 2023
This is based on this PR: withastro/astro#6453 which landed a few months ago.
mooreds added a commit to FusionAuth/fusionauth-site that referenced this pull request Aug 16, 2023
This is based on this PR: withastro/astro#6453 which landed a few months ago.
alex-fusionauth pushed a commit to FusionAuth/fusionauth-site that referenced this pull request Sep 5, 2023
This is based on this PR: withastro/astro#6453 which landed a few months ago.
alex-fusionauth added a commit to FusionAuth/fusionauth-site that referenced this pull request Sep 22, 2023
* initial install

* update

* pr fixes

* remove type file

* pr updates

* remove photo refs

* Revert "disable basic auth for testing (#2442)"

This reverts commit f34339f.

* Revert "disable basic auth for testing (#2442)"

This reverts commit f34339f.

* add opengraph image

* reauth dev

* don't limit header image

* article to education

* bradm publishes inc5000 blog post (#2452)

* bradm publishes inc5000 blog post

* fixed an H2

* Fixed the absolute URLs

---------

Co-authored-by: Brad McCarty <[email protected]>

* fixing the inc5000 placement (#2454)

Co-authored-by: Brad McCarty <[email protected]>

* removing extra / at end of link (#2455)

* 1.47 release announcement didn't get correct category, prob due to when it was published

* add in supabase (#2456)

* spellcheck astro site too (#2457)

* removed extra space from URL

* tbd for spellcheck

* updated with more known words

* uninstall compress so spellcheck runs faster

* Migrated a missed post, couple of fixups

* pre-compressing images

* better blurbage

* Header and footer changes

* trailing slash on blog pages

* oops

* oops

* oops...

* fix for latest

* fix extra space (#2458)

* fixing extra space

* Mooreds/update spellcheck for astro (#2459)

* make sure we catch all astro files for spellcheck

* Make sure get all astro html, and exclude the ESC button on devtools

Note that this currently picks up a lot of truncated words, so don't fix this until the blurbing works better

* Add fusiondesk post (#2380)

Adds a react SDK example blog post.

---------

Co-authored-by: Colin Frick <[email protected]>

* No trailing slash for blog items. (#2460)

This is based on this PR: withastro/astro#6453 which landed a few months ago.

* Mooreds/update content check (#2462)

* updated content check to look for categories and check titles

* fix category of this post

* Mooreds/fix cats blog post (#2463)

* corrected tags

* corrected category

* commented out title check until Brad decides

* don't worry about compress right now (#2464)

* Bradm/vc free (#2465)

* pushing this up for Lyle to fix

* small updates

* brad publishes VC Free post

* fixed category failure

---------

Co-authored-by: Brad McCarty <[email protected]>

* Mooreds/fix spelling aug 17 2023 (#2466)

* added more known words

* corrected spelling errors or typos

* Mooreds/check for absolute urls (#2467)

* check for absolute fusionauth.io urls and empty markdown links `[text...]()`.

we want to use relative urls

* Mooreds/1951 upgrade guide (#2461)

Upgrade guide overhaul

* Mooreds/add color about downtime (#2468)

* added note about downtime expectations

* Revise docsdevreadme for new world (#2471)

* Revise docsdevreadme for new world

* emphasized header stuff

* added note about start case

* trying to fix linkcheck (#2472)

we get a lot of connection failure errors

* added note about links

* Mooreds/fix 404s from linkcheck (#2473)

* fixing images

* handling another 404 image

* handling 404 images

* Image URL wasn't correctly updated

* fixed unmoved image

* fixed busted link

* correct sitemap submission params

* only run 1/day

* Mooreds/fix spelling aug 18 2023 (#2474)

* ignore tag files

* fixed typos

* more known words

* Docs & dev readme style grammar fix (#2475)

Co-authored-by: worktheclock <[email protected]>

* add id for article (#2469)

* clean out the s3 bucket on deploy

* fix markdown that made it into asciidoc (#2476)

* Expanded 2xl dimension on index page for sidebar

* Updated nextjs header image

* Update index footer blurb text

* clean out the s3 bucket on deploy

* Document options for change password during a slow migration (#2477)

* Capturing some knowledge that came up during a support discussion

* minor formatting change

* exclude stuff that isn't "content" from the search

* oops

* debounce the search and only update the dom when done

* fetch faster

* fix merge mishap

* fix busted image (#2479)

* add 504 response code for /api/two-factor/login (#2480)

* clarifying doc (#2481)

* fixed breaking external links (#2482)

* fixed breaking external links

* fixed breaking external tag links

* redirect old category names

* fix busted link

* tagging additional conference posts (#2483)

* add note about CLI goals (#2484)

* brad publishes mark's unlimited domains post (#2485)

Co-authored-by: Brad McCarty <[email protected]>

* updated unlimited custom domain header style

* Updated diagram img path

* updated unlimited domain diagram

* update to img rounded corners

* fix the blog blog link

* remove nbsps as they did not look on-purpose

* Add notes about loginId

* Blog width update

* Fixing the changebank link (#2487)

Want to send folks to the right parody video

* Update sitemap.xml

Compare Auth0 page going live on brochure site.

* fix changebank logo, added multiplattform font definitions. (#2489)

* Added link to quickstarts (#2490)

* Make it clearer which webhook types are for premium, enterprise and community plans (#2446)

* Add treefort interview (#2414)

* initial import

* spell check

* rename

* updating header image

* shrinking image

* fixing name mispelling

* changing title per SEO review

* prepping treefort interview

* changing title

* added some links

* fixed tags and categories

* Use new image

* fixed post date, incorrect tag

* Quickstart django astro (#2443)

* Added astro django menu entry for article

* Correct Rails to Flask typo

* Change references to new quickstart doc everywhere

* Work on python doc

* Getting old django quickstart to work

* Old django quickstart now works complete in astro with code. Now to make it meet the new standard

* English

* English

* Finished draft

* text updates

* text updates

* Python Django Quickstart language edit

* Python Django Quickstart proofread

* Update astro/src/content/quickstarts/quickstart-python-django-web.mdx

Co-authored-by: Dan Moore <[email protected]>

* Change we to you

* remove the django guide example from the example apps data file

* Add rediretion for new guide

* Remove filename above code snippets

* Remove ; from shell commands

* Change absolute links to relative

* Name file

* Remove integrate-python-django.adoc

* Change from title case to start case

* Tell Dan we don't need to cd at any point

* Explain why we are using venv

* Remove unnecessary command to start Django server

* Clarified all commands in directories

* Comments on windows

* Add warning about passwords in git

* fix relative urls

* Revised Python Django Quickstart language edit

* Apply suggestions from code review

Co-authored-by: Dan Moore <[email protected]>

* update Elastic to Elasticsearch

---------

Co-authored-by: RichardJECooke <[email protected]>
Co-authored-by: Tatenda <[email protected]>
Co-authored-by: worktheclock <[email protected]>
Co-authored-by: Dan Moore <[email protected]>

* bradm blog SEO updates (#2496)

Co-authored-by: Brad McCarty <[email protected]>

* adding color to google custom parameters section (#2497)

* fix busted include (#2498)

* add in angular sdk (#2499)

* SCIM server doc updates (#2501)

Added more details, fixed some inconsistencies.

* add 504 webhook login response code to /api/login, /api/passwordless/login and /api/identity-provider/login endpoint docs (#2495)

* Mcr/blog get more value (#2312)

* get more value post

* updating link to sample application

* updating with changes from Brad

* Update site/_posts/2023-07-11-get-more-value-out-of-fusionauth.md

reformat paragraph

Co-authored-by: Dan Moore <[email protected]>

* update links verbiage

Co-authored-by: Dan Moore <[email protected]>

* Update conclusion verbiage

Co-authored-by: Dan Moore <[email protected]>

* Update Link verbiage

Co-authored-by: Dan Moore <[email protected]>

* making changes from review

* finishing merge

* small clean ups

* making changes from review

* update word

* Update site/_posts/2023-07-11-get-more-value-out-of-fusionauth.md

update Burger sp

Co-authored-by: Dan Moore <[email protected]>

* Update site/_posts/2023-07-11-get-more-value-out-of-fusionauth.md

updating language

Co-authored-by: Dan Moore <[email protected]>

* Update site/_posts/2023-07-11-get-more-value-out-of-fusionauth.md

Update Language

Co-authored-by: Dan Moore <[email protected]>

* updating typos

* adding to example-apps

* removing Net language and capatilizing blog post headers.

* moving to astro

* tweaking for display

* update categories

* updating date and image

* removing old images

---------

Co-authored-by: Dan Moore <[email protected]>

* fixing tag that was preventing build (#2504)

* Update image for Value

* updates based on Dan's PR Review

* add makechange

* correct merge conflict

* update makechange

* add Login Link

---------

Co-authored-by: Andy Pai <[email protected]>
Co-authored-by: Lyle Schemmerling <[email protected]>
Co-authored-by: Brad McCarty <[email protected]>
Co-authored-by: Brad McCarty <[email protected]>
Co-authored-by: Dan Moore <[email protected]>
Co-authored-by: Colin Frick <[email protected]>
Co-authored-by: Gareth Dwyer <[email protected]>
Co-authored-by: worktheclock <[email protected]>
Co-authored-by: Sean Bryant <[email protected]>
Co-authored-by: Brent Halsey <[email protected]>
Co-authored-by: Daniel DeGroff <[email protected]>
Co-authored-by: Sean Bobby <[email protected]>
Co-authored-by: Michael Roth <[email protected]>
Co-authored-by: RichardJECooke <[email protected]>
Co-authored-by: Tatenda <[email protected]>
Co-authored-by: Mark Robustelli <[email protected]>
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.

RSS integration does not respect trailingSlash configuration option
4 participants