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
feature/add-catch-all-route #223
feature/add-catch-all-route #223
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I have few minor asks for the styling.
Just a heads up regarding git history. Your local main
should contain only peer-reviewed code. Meaning you shouldn't merge local changes to your local main
branch. Instead, you can get the latest changes from upstream (this repo)'s main
branch if you need to resolve a merge conflict. This way you can keep the commit history of the PR clean (e.g. it wouldn't include your Underline commit in this PR).
Edit:
Or maybe you created a new branch(enhancement/addcatchallroute) based off your "Underline" branch instead of the main
branch.
Great suggestions To. Neel, super pleased to see your work 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I paired with Neel this afternoon! Great fun, glad he's on the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So happy this is in! Agree with To that we should try to use a mozilla styled button but otherwise. I am pretty happy with this. I leave code comments to To and Brian
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes.
Link to Relevant Issue
This pull request resolves #102
Description of Changes
Displays an error message when the route given is not provided. Also provides a general error page for future development.
Link to Site
https://neelapte.github.io/cdp-frontend/#/9999
This page displays a page not found error.
https://neelapte.github.io/cdp-frontend/#/events/8888
This page displays a formatted error.
Please see
CONTRIBUTING.md
for directions on how this can be done.