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

docs(getting-started): update readme.md and bootstrap.md files #3933

Merged
merged 10 commits into from Apr 5, 2018
Merged

docs(getting-started): update readme.md and bootstrap.md files #3933

merged 10 commits into from Apr 5, 2018

Conversation

ghost
Copy link

@ghost ghost commented Mar 6, 2018

It was done in readme.md:

  • updated Installation instructions
    (add info about adding packages and how to add component in html)
  • updated How to build lib for development
    (add 'Keeping a fork up to date' section)
  • updated Headers style
    (for semantics, first heading is h1, other - h2 and below )
  • deleted licence link from Table of contents list but add it to the end of the page
    (agreed at the meeting - as usual licence is added at the end of the readme page)
  • join together angular-cli and Bootstrap 4 and angular-cli sections to Bootstrap 3 or 4 and angular-cli in the How to use it with section
    (because they has similar structure and goals)

Also was decided ( at the meeting ) to update bootstrap.md files:

  • rename "testing" to "checkout"
    (this name is more logic, because this section doesn't describe how to add tests)
  • add example with adding css style to index.html
    (for better understanding how user can add css styles)
  • move bootstrap versions from main title to text + change file's name from bootstrap4 to bootstrap
    (because this file not only for bootstrap 4, it's an instruction how to use bootstrap 3 and bootstrap 4 with angular-cli)

closes #3934

@ghost ghost changed the title docs(documentation): update readme and bootstrap.md files docs(readme): update readme.md and bootstrap.md files Mar 6, 2018
@ghost
Copy link
Author

ghost commented Mar 6, 2018

fix button_page_spec to run test

@codecov
Copy link

codecov bot commented Mar 6, 2018

Codecov Report

Merging #3933 into development will decrease coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #3933      +/-   ##
===============================================
- Coverage        72.23%   72.17%   -0.07%     
===============================================
  Files              253      253              
  Lines             8238     8238              
  Branches          1565     1565              
===============================================
- Hits              5951     5946       -5     
- Misses            1866     1868       +2     
- Partials           421      424       +3
Impacted Files Coverage Δ
src/chronos/i18n/it.ts 71.42% <0%> (-28.58%) ⬇️
src/chronos/i18n/pl.ts 72.22% <0%> (-5.56%) ⬇️
src/chronos/i18n/uk.ts 78.04% <0%> (-2.44%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8ee1f8...d42161b. Read the comment docs.

README.md Outdated
```bash
npm install ngx-bootstrap --save
```

You will need bootstrap styles (Bootstrap 3)
Add needed package to AppModule imports:

This comment was marked as off-topic.

@@ -66,67 +66,68 @@ describe('Buttons page test suite', () => {
buttons.clickByText(buttonDemos[2], buttonNames[1]);

This comment was marked as off-topic.

- Open the file .angular-cli.json from the root of your project.
- Under the property apps the first item in that array is the default application.
- There is a property styles which allows external global styles to be applied to your application.
Now that the project is set up it must be configured to include the Bootstrap CSS. You have two choices:

This comment was marked as off-topic.

"component": {}
}
"styles": [
"styles.scss" <-- rename this from .css to .scss

This comment was marked as off-topic.

.
.
"defaults": {
"styleExt": "scss", <-- set this to default to .scss

This comment was marked as off-topic.

.
"defaults": {
"styleExt": "scss", <-- set this to default to .scss
"component": {}

This comment was marked as off-topic.

valorkin
valorkin previously approved these changes Mar 13, 2018
IlyaSurmay
IlyaSurmay previously approved these changes Mar 14, 2018
IlyaSurmay
IlyaSurmay previously approved these changes Mar 15, 2018
@YevheniiaMazur
Copy link

Please, update PR and task names, cause now it looks like two features in one PR.
And also please, add descriptions to checklist what was done (in issue or in PR), not clear why we need several of them

@ghost ghost changed the title docs(readme): update readme.md and bootstrap.md files docs(getting-started): update readme.md and bootstrap.md files Mar 16, 2018
@ghost
Copy link
Author

ghost commented Mar 19, 2018

@YevheniiaMazur fixed

@YevheniiaMazur
Copy link

YevheniiaMazur commented Mar 20, 2018

@svetoldo4444ka , should I directly read these files, or they affected demo or other structure objects? Is these files are content of Getting started page?

@YevheniiaMazur
Copy link

Several fixes/clarifyings needed:

  1. Link to bootstrap.md file at README file throws 404 error:
    404link

  2. Why we need command npm run test for updating fork?
    npmruntest

  3. Why we need latest stable version of angular for using angular-cli with bootstrap? If I'm not wrong, it'll work with previous versions too
    lateststable

  4. Maybe will be better use flag --save-dev (to add bootstrap directly to project's devDependencies) or any flag, written at npm documentation instead of --save? cause in npm documentation there are no examples of using flag --save without suffix
    saveflag
    link to documentation: https://docs.npmjs.com/cli/install

  5. Why we need duplicating info in README file
    readme
    And in bootstrap.md file
    checkout

@ghost
Copy link
Author

ghost commented Mar 22, 2018

@YevheniiaMazur @EvilAlexei @v1ktoryy
According to meeting:

  • 3 and 5 list items was decided to retain
  • 4 list items will be fixed
  • 2 item:
  1. was decided to change title
    joxi_screenshot_1521725322500
  2. add stepd to
    http://joxi.ru/L216zjet6d1dbA
  3. add comment for
    joxi_screenshot_1521725536789

according to 1 comment:
Link to bootstrap.md file at README file throws 404 error because this file isn't exist in this branch while it isn't pushed in development branch. And now I can't check it

README.md Outdated
```bash
npm install ngx-bootstrap --save
npm install ngx-bootstrap --save-dev

This comment was marked as off-topic.

README.md Outdated
## How to use it with:
- `angular-cli` please refer to [getting-started-with-ng-cli](https://github.com/valor-software/ngx-bootstrap/tree/development/docs/getting-started/ng-cli.md)
### How to use it with:
- `Bootstrap 3 or 4 and angular-cli` please refer to [using-with-bootstrap-and-angular-cli](https://github.com/valor-software/ngx-bootstrap/tree/development/docs/getting-started/bootstrap.md)

This comment was marked as off-topic.

- install `ngx-bootstrap` and `Bootstrap`

```bash
npm install ngx-bootstrap bootstrap --save-dev

This comment was marked as off-topic.

@valorkin
Copy link
Member

@svetoldo4444ka apply code review comments

@ghost
Copy link
Author

ghost commented Mar 29, 2018

@valorkin @EvilAlexei fixed according to the comments( commit - docs(getting-started): update readme.md and bootstrap.md files according to the comments)

@ghost
Copy link
Author

ghost commented Mar 29, 2018

@YevheniiaMazur @EvilAlexei
according to @valorkin comments:

  • returned command npm install ngx-bootstrap --save
  • changed
    joxi_screenshot_1522331504954
    To
    joxi_screenshot_1522331518018

@ghost ghost added the in progress label Mar 30, 2018
README.md Outdated
- `git pull upstream development`
- `rm -rf node_modules`
- `npm install`
- `npm run test` _*// it will build the lib, create a link in node_modules and run package's "test" script*_

To run bootstrap 3 and 4 demo:

This comment was marked as off-topic.

@ghost ghost added the in progress label Apr 2, 2018
@YevheniiaMazur
Copy link

Tested, looks good.
Link to bootstrap.md file at README file should be retested after merging PR, due to reason answered before:
404explanation

@valorkin valorkin merged commit 621e8dd into valor-software:development Apr 5, 2018
@ghost ghost removed the ready for merge label Apr 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs(getting-started): update readme.md file
3 participants