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: added support for example short urls #3311

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

aakankshabhende
Copy link
Contributor

@aakankshabhende aakankshabhende commented Oct 21, 2024

Description
Added support for example short urls. Fixes #3005

Related issue(s)

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced new redirection rules for short URLs, enhancing user access to specific GitHub links.
    • Updated redirection paths for documentation, consolidating tutorials under a single location.
    • Expanded redirection for AsyncAPI specifications to include multiple versions.
    • Added redirections for Slack invites and verification links for Maven and Gradle repositories.
  • Bug Fixes

    • Ensured users are directed to the most relevant and updated resources through improved redirection organization.

Copy link

coderabbitai bot commented Oct 21, 2024

Walkthrough

The changes made in the _redirects file involve the addition of new redirection rules and the restructuring of existing ones. New rules include specific short URL redirects to GitHub links and a generalized pattern for similar links. Existing documentation paths have been updated to direct users to new tutorial locations, while the AsyncAPI specifications have been expanded to include multiple version redirects. Other updates include maintaining the redirect for the old React Playground and adding new links for Slack invites and Maven/Gradle repository verification.

Changes

File Change Summary
public/_redirects - Added new redirection rules for short URLs: /s/example and /s/* to GitHub links.
- Updated existing documentation paths to point to /docs/tutorials/getting-started/.
- Expanded AsyncAPI specifications redirection for multiple versions to their GitHub documentation.
- Retained redirect for old React Playground to new AsyncAPI React documentation.
- Added redirects for Slack invites and Maven/Gradle repository verification links.

Assessment against linked issues

Objective Addressed Explanation
Support short URLs (#3005)

Suggested labels

ready-to-merge

Suggested reviewers

  • derberg
  • sambhavgupta0705
  • akshatnema
  • anshgoyalevil
  • magicmatatjahu

Poem

In the land of links where rabbits play,
New paths are crafted, brightening the way.
From short URLs to tutorials anew,
We hop with joy, for the changes are true!
Redirects like carrots, fresh and so bright,
Guiding our friends to resources just right! 🐇✨


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 04f703f and 6ab0b2b.

📒 Files selected for processing (1)
  • public/_redirects (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • public/_redirects

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Oct 21, 2024

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 6ab0b2b
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/6757f1b41360c00008c32c40
😎 Deploy Preview https://deploy-preview-3311--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
public/_redirects (2)

73-77: LGTM! Consider adding a comment for clarity.

The new short URL redirections are well-implemented and align with the PR objective. They provide a convenient way to access example AsyncAPI specifications.

Consider adding a brief comment above the SHORT-URL-REDIRECTION section to explain its purpose. For example:

+# Short URL redirections for quick access to AsyncAPI examples
 # SHORT-URL-REDIRECTION:START
 /s/example https://github.com/asyncapi/spec/blob/master/examples/streetlights-mqtt-asyncapi.yml 301!
 /s/* https://github.com/asyncapi/spec/blob/master/examples/:splat 301!
 # SHORT-URL-REDIRECTION:END

Potential Conflict with New /s/* Redirection

The existing redirection /s/example may conflict with the new /s/* wildcard rule.

  • public/_redirects, Line: /s/example https://github.com/asyncapi/spec/blob/master/examples/streetlights-mqtt-asyncapi.yml 301!
🔗 Analysis chain

Line range hint 1-77: Verify no conflicts with existing redirections.

The placement of the new short URL redirections at the end of the file is correct. However, it's important to ensure that no existing redirections conflict with the new /s/* wildcard rule.

Run the following script to check for potential conflicts:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential conflicts with the new /s/* redirection rule

# Test: Search for any existing rules that might conflict with /s/*
rg '^/s/' public/_redirects | grep -v '^/s/\*'

# If the above command returns any results, it indicates a potential conflict
# that needs to be addressed.

Length of output: 149

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 3a8f69a and b03539c.

📒 Files selected for processing (1)
  • public/_redirects (1 hunks)
🧰 Additional context used

@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Oct 21, 2024

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 46
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

Lighthouse ran on https://deploy-preview-3311--asyncapi-website.netlify.app/

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
public/_redirects (1)

73-77: LGTM! Consider placement of wildcard redirection.

The addition of short URL redirections is well-implemented and aligns with the PR objective. The use of a comment block for easy identification is a good practice.

A minor suggestion:

Consider moving the wildcard redirection /s/* above the specific /s/example redirection. This ensures that more specific rules take precedence over the wildcard rule, following the principle of most specific to least specific ordering in redirection rules.

 # SHORT-DOC-URL-REDIRECTION:START
+/s/* https://github.com/asyncapi/spec/blob/master/examples/:splat 301!
 /s/example https://github.com/asyncapi/spec/blob/master/examples/streetlights-mqtt-asyncapi.yml 301!
-/s/* https://github.com/asyncapi/spec/blob/master/examples/:splat 301!
 # SHORT-DOC-URL-REDIRECTION:END

This change would prevent any potential conflicts if more specific short URLs are added in the future.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between b03539c and e095b44.

📒 Files selected for processing (1)
  • public/_redirects (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
public/_redirects (1)

Line range hint 1-77: Overall, the changes look good and achieve the PR objective.

The addition of short URL redirections for examples is well-implemented and doesn't interfere with existing redirections. The use of a comment block for easy identification is a good practice. With the minor suggestion for rule ordering, this implementation provides a solid foundation for managing example short URLs.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
public/_redirects (1)

73-77: LGTM! Consider optimizing rule order.

The new redirection rules for short URLs are well-implemented:

  1. The wildcard rule /s/* provides a flexible way to access various examples.
  2. The specific rule for /s/example offers a quick link to a common example.

For optimal routing, consider swapping the order of these rules:

# SHORT-DOC-URL-REDIRECTION:START
-/s/* https://github.com/asyncapi/spec/blob/master/examples/:splat 301!
 /s/example https://github.com/asyncapi/spec/blob/master/examples/streetlights-mqtt-asyncapi.yml 301!
+/s/* https://github.com/asyncapi/spec/blob/master/examples/:splat 301!
# SHORT-DOC-URL-REDIRECTION:END

This ensures the specific rule takes precedence over the wildcard rule, maintaining the intended behavior while potentially improving routing efficiency.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e095b44 and 637f42f.

📒 Files selected for processing (1)
  • public/_redirects (1 hunks)
🧰 Additional context used

@aakankshabhende
Copy link
Contributor Author

@coderabbitai full review

Copy link

coderabbitai bot commented Nov 11, 2024

✅ Actions performed

Full review triggered.

@aakankshabhende
Copy link
Contributor Author

@akshatnema could you please review

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
public/_redirects (1)

74-77: Consider adding documentation for short URLs.

Since this is a new feature for short URLs, it would be helpful to document:

  1. The purpose and usage of these short URLs
  2. Examples of valid short URL patterns
  3. Guidelines for adding new short URLs

Would you like me to help create documentation for the short URL feature?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 3a8f69a and 637f42f.

📒 Files selected for processing (1)
  • public/_redirects (1 hunks)
🔇 Additional comments (2)
public/_redirects (2)

73-73: LGTM: Empty line provides clear section separation.

The empty line helps maintain readability by separating different redirect rule sections.


74-77: Verify the example URLs are accessible.

Let's ensure that the target URLs for the new redirect rules are valid and accessible.

✅ Verification successful

Both redirect URLs are accessible and valid

The verification results show that both URLs are accessible:

  • The example YAML file (streetlights-mqtt-asyncapi.yml) returns a 200 status code
  • The examples directory URL returns a 301 redirect to the correct tree view URL, which is expected and valid
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the target URLs for the new redirect rules are accessible

# Test 1: Check if the example YAML file exists
echo "Checking streetlights-mqtt-asyncapi.yml..."
curl -I https://github.com/asyncapi/spec/blob/master/examples/streetlights-mqtt-asyncapi.yml

# Test 2: Check if the examples directory exists
echo -e "\nChecking examples directory..."
curl -I https://github.com/asyncapi/spec/blob/master/examples/

Length of output: 10051

public/_redirects Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
public/_redirects (1)

74-77: Consider adding documentation for the short URL feature.

Since this is a new feature, it would be helpful to add a comment explaining:

  • The purpose of these short URLs
  • Examples of how to use them
  • What types of examples are available

Example comment:

 # SHORT-DOC-URL-REDIRECTION:START
+# Short URLs for AsyncAPI examples:
+# - /s/example -> Points to the streetlights MQTT example
+# - /s/{example-name} -> Points to other examples in the spec repo
 /s/example https://github.com/asyncapi/spec/blob/master/examples/streetlights-mqtt-asyncapi.yml 301!
 /s/* https://github.com/asyncapi/spec/blob/master/examples/:splat 301!
 # SHORT-DOC-URL-REDIRECTION:END
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 637f42f and 04f703f.

📒 Files selected for processing (1)
  • public/_redirects (1 hunks)
🔇 Additional comments (1)
public/_redirects (1)

74-77: LGTM! Correct implementation of short URL redirects.

The implementation looks good with:

  • Proper rule ordering (specific before wildcard)
  • Appropriate use of 301 permanent redirects
  • Correct usage of :splat parameter

Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.22%. Comparing base (7f81142) to head (6ab0b2b).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3311   +/-   ##
=======================================
  Coverage   77.22%   77.22%           
=======================================
  Files          21       21           
  Lines         663      663           
=======================================
  Hits          512      512           
  Misses        151      151           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -70,3 +70,8 @@ https://www.asyncapi.io/* https://www.asyncapi.com/:splat 301!
# Additional redirection
/community/meetings /community/events 301!
/cheatsheet https://github.com/asyncapi/website/tree/master/cheatsheet 302!

# SHORT-DOC-URL-REDIRECTION:START
/s/example https://github.com/asyncapi/spec/blob/master/examples/streetlights-mqtt-asyncapi.yml 301!
Copy link
Member

Choose a reason for hiding this comment

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

It should be 302 (temporary redirect), as we might want to change the destination of short URLs. 301 would ask the browser to cache destination URL.

@anshgoyalevil
Copy link
Member

Thanks @aakankshabhende
The solution does work perfectly, but I am not sure if this is the level of flexibility we want in implementing the short URLs. We would need to create a commit to this _redirects file each time we want to create a new short URL.

If that level of manual work is fine, @derberg?, let's go ahead with this approach for future short URLs

@sambhavgupta0705
Copy link
Member

/update

@sambhavgupta0705
Copy link
Member

@akshatnema @derberg PTAL

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.

support short urls
4 participants