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

Fixing the typos and grammatical errors in Readme files #1246

Merged
merged 2 commits into from
Feb 23, 2020
Merged

Fixing the typos and grammatical errors in Readme files #1246

merged 2 commits into from
Feb 23, 2020

Conversation

mohitm15
Copy link
Contributor

What kind of change does this PR introduce?

Bug Fix, fixes the issue #1245

If relevant, did you update the documentation?
Yes, Security.md and webpack-scaffold/readme.md

Summary
Fixing the typos and grammatical errors in Readme files

Does this PR introduce a breaking change?
No

Other information

@mohitm15 mohitm15 requested a review from a team as a code owner February 21, 2020 13:01
@jsf-clabot
Copy link

jsf-clabot commented Feb 21, 2020

CLA assistant check
All committers have signed the CLA.

@ghost
Copy link

ghost commented Feb 21, 2020

There were the following issues with this Pull Request

  • Commit: 1c29100
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@webpack-bot
Copy link

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

Copy link
Member

@anshumanv anshumanv left a comment

Choose a reason for hiding this comment

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

Your commit is not as expected, please commit using npm run commit or yarn commit

@mohitm15
Copy link
Contributor Author

mohitm15 commented Feb 21, 2020

I uses git commit -m (message) @anshumanv

Sent from my VIBE K6 Power using FastHub

@anshumanv
Copy link
Member

We have specific commit guidelines which are most easily to adhere to using the commit helper package that we use, just do npm run commit and follow the prompt then you should be good.

@@ -86,7 +86,7 @@ this.configuration.myScaffold.webpackOptions.entry = createDynamicPromise(["app.

Param: `String`

Used to create an [assetFilterFunction](https://webpack.js.org/configuration/performance/#performance-assetfilter)
Used to create a [assetFilterFunction](https://webpack.js.org/configuration/performance/#performance-assetfilter)
Copy link
Member

Choose a reason for hiding this comment

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

The former was fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See its readme context

The former was fine.

Screenshot 2020-02-22 13:28:31

Copy link
Member

Choose a reason for hiding this comment

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

@mohitm15 you should be making the change here

Copy link
Member

Choose a reason for hiding this comment

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

Instead you made the change here:
Screenshot from 2020-02-22 14-54-43

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for correcting @jamesgeorge007

@@ -86,7 +86,7 @@ this.configuration.myScaffold.webpackOptions.entry = createDynamicPromise(["app.

Param: `String`

Used to create an [assetFilterFunction](https://webpack.js.org/configuration/performance/#performance-assetfilter)
Copy link
Member

Choose a reason for hiding this comment

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

I think an is correct.

Copy link
Contributor Author

@mohitm15 mohitm15 Feb 22, 2020

Choose a reason for hiding this comment

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

See above conversation, self-explanatory

@anshumanv
Copy link
Member

Also please sign the CLA.

Copy link
Contributor

@wizardofhogwarts wizardofhogwarts left a comment

Choose a reason for hiding this comment

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

It seems to be that the change made is not what is pointed to in the screenshot shared. @mohitm15 You should revert the change you made and make the change where you shown in the screenshot. That's correct. @rishabh3112 is requested to review.

@mohitm15
Copy link
Contributor Author

We have specific commit guidelines which are most easily to adhere to using the commit helper package that we use, just do npm run commit and follow the prompt then you should be good.

Getting error logs while running it

@anshumanv
Copy link
Member

Getting error logs while running it

What errors? Trace?

@webpack-bot
Copy link

@mohitm15 Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@wizardofhogwarts Please review the new changes.

@ghost
Copy link

ghost commented Feb 22, 2020

There were the following issues with this Pull Request

  • Commit: 1c29100
    • ✖ message may not be empty
    • ✖ type may not be empty
  • Commit: 73b12eb
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@mohitm15
Copy link
Contributor Author

Getting error logs while running it

What errors? Trace?

Screenshot 2020-02-22 15:58:17

@anshumanv
Copy link
Member

Did you install the project dependencies?

@ematipico
Copy link
Contributor

commit a command of our CLI :)

Maybe you wanted to do git commit

@mohitm15
Copy link
Contributor Author

mohitm15 commented Feb 23, 2020

Did you install the project dependencies?

NO , can you direct me to the link of project setup on local machine

@anshumanv
Copy link
Member

@mohitm15 - https://github.com/webpack/webpack-cli/blob/next/.github/CONTRIBUTING.md

Read the Docs 😄

ematipico
ematipico previously approved these changes Feb 23, 2020
@rishabh3112
Copy link
Member

Thanks @mohitm15

@mohitm15
Copy link
Contributor Author

@mohitm15 - https://github.com/webpack/webpack-cli/blob/next/.github/CONTRIBUTING.md

Read the Docs

@anshumanv I am facing a problem regarding comminting the changes for my next PR. Can we have chat over gitter channel

@anshumanv
Copy link
Member

anshumanv commented Feb 29, 2020

@anshumanv I am facing a problem regarding comminting the changes for my next PR. Can we have chat over gitter channel

Sure we can, what's the problem exactly? Doing a discussion here will also help anyone who is having similar issues so I'd recommend doing it here.

@mohitm15
Copy link
Contributor Author

@anshumanv I am facing a problem regarding comminting the changes for my next PR. Can we have chat over gitter channel

Sure we can, what's the problem exactly? Doing a discussion here will also help anyone who is having similar issues so I'd recommend it doing here.

Sure

I had made changes in a separate branch for my latest issue, but after adding the changes using git add <filename> , I run git commit -m <message> but its not working, so I then tried yarn commit, but still causing error

@anshumanv
Copy link
Member

I had made changes in a separate branch for my latest issue, but after adding the changes using git add , I run git commit -m but its not working, so I then tried yarn commit, but still causing error

What's the error? Are you sure you did yarn install?

@mohitm15
Copy link
Contributor Author

I had made changes in a separate branch for my latest issue, but after adding the changes using git add , I run git commit -m but its not working, so I then tried yarn commit, but still causing error

What's the error? Are you sure you did yarn install?

Yes

@snitin315
Copy link
Member

snitin315 commented Feb 29, 2020

refer #1267 .

Make sure your branch is up-to-date with upstream and have this commit .

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.

9 participants