| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
PatternFly-React preview: https://patternfly-react-pr-3254.surge.sh |
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #3254 +/- ##
=======================================
Coverage 67.43% 67.43%
=======================================
Files 892 892
Lines 24869 24869
Branches 2140 2140
=======================================
Hits 16770 16770
Misses 7094 7094
Partials 1005 1005
Continue to review full report at Codecov.
|
Sorry, something went wrong.
There was a problem hiding this comment.
👍
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for doing this! I left a couple of questions/suggestions.
Sorry, something went wrong.
|
@seanforyou23 Thank you for your helpful feedback Michael! I've updated the README according to your suggestions. |
Sorry, something went wrong.
| ``` | ||
|
|
||
| Go to localhost:8000. | ||
|
|
There was a problem hiding this comment.
Although adding more info to the README is a good thing, this does not appear to address the original issue? That is, the yarn start:dev command no longer confirms exactly where the app is running.
This also does not account for the example app running on a different port, should port 8000 already be occupied.
Sorry, something went wrong.
There was a problem hiding this comment.
@dlabrecq Thank you for your feedback Dan! This has to do with something Zack addressed here: #3154 (comment) and I added that line based on what was agreed upon in this comment. But I see what you mean -- this PR won't necessarily close that issue.
Good point about the different port -- do you happen to know what port the app runs on if localhost:8000 is already occupied?
Sorry, something went wrong.
There was a problem hiding this comment.
@jenny-s51 if the port is already in use it prints to the console @patternfly/react-docs: Something is already running at port 8000. I think it used to be the case that an interactive console would take over and allow you to specify a different port, then proceed to launch the dev server using that new config. It seems to have regressed or been removed at some point.
I'm personally ok with pulling these changes in, but I agree it doesn't really fix the root cause. So, as long as we can leave the related issue open for future fixes we should be ok. @dlabrecq does this sound ok to you?
Sorry, something went wrong.
There was a problem hiding this comment.
Ok, can leave that issue opened
Sorry, something went wrong.
There was a problem hiding this comment.
👍
Sorry, something went wrong.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Temporary fix for #3154 (comment)