FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

PF4: fix(README.md): provides URL location of workspace by jenny-s51 · Pull Request #3254 · patternfly/patternfly-react · GitHub

/ patternfly-react Public

PF4: fix(README.md): provides URL location of workspace #3254

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.

Sign up for GitHub

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

Merged
seanforyou23 merged 2 commits into from
Nov 6, 2019
Conversation 13 Commits 2 Checks 0 Files changed
Merged

PF4: fix(README.md): provides URL location of workspace #3254

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
05fc78d
update README.md
jenny-s51 Nov 1, 2019
0982d08
update readme.md
jenny-s51 Nov 4, 2019
File filter

Filter by extension

Filter by extension .md  (1) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
8 changes: 5 additions & 3 deletions packages/patternfly-4/react-core/README.md
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,25 @@ Note: All commands below assume you are on the root directory in this repository

### Install

Run to install all the dependencies
Run to install all the dependencies.

```sh
yarn install && yarn build
```

### Running

To start the site locally.
To launch the development server and view the workspace:

```sh
yarn start:pf4
```

Go to localhost:8000.

dlabrecq Nov 5, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

jenny-s51 Nov 5, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@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?

seanforyou23 Nov 5, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@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?

dlabrecq Nov 6, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ok, can leave that issue opened
### Building

To build the site.
To build the site:

```sh
yarn build:docs
Expand Down
Back | FazBrowse Home | New Git URL