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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: change hasone relation error message #4290

Merged

Conversation

sujeshthekkepatt
Copy link
Contributor

@sujeshthekkepatt sujeshthekkepatt commented Nov 30, 2019

The current hasone error message is not appropriate one.
So adds a better message.
See more #4278

Checklist

馃憠 Read and sign the CLA (Contributor License Agreement) 馃憟

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • Commit messages are following our guidelines

@slnode
Copy link

slnode commented Nov 30, 2019

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@sujeshthekkepatt
Copy link
Contributor Author

Sorry for the closed PR's some misunderstanding occurred.

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

Thank you @sujeshthekkepatt for the pull request.

It looks like you have reformatted all code in the two files you are modified, this makes it pretty much impossible to review the changes. Please revert the formatting changes and modify only the relevant lines.

I am guessing that your editor/IDE is configured to automatically format the code on save, maybe using Prettier? You should disable that feature while working in LoopBack 3 codebase.

Also please keep all work in this pull request, DO NOT open a new one. We can help you to clean the git commit history once the changes look good.

You can run git revert HEAD or git revert -n HEAD to undo the formatting changes.

@bajtos bajtos self-assigned this Dec 2, 2019
@sujeshthekkepatt
Copy link
Contributor Author

@bajtos thanks for the review. Sorry for the formatting problem. I will modify accordingly. I got a confusion on the linter and prettier. That's what happened.

@sujeshthekkepatt
Copy link
Contributor Author

@bajtos seems there is a commit linter issue. Can you check the code now?

@dhmlau
Copy link
Member

dhmlau commented Dec 2, 2019

@sujeshthekkepatt, the commit linter error is:

**************************************************
**
**  Linting commit logs
**
**  1 problems found:
**    ec1d4ce - Revert "feat: change hasone relation error message": First line should be 50 characters or less (saw 51)
**
**************************************************

For details, please refer to our commit message guideline. Thanks! https://loopback.io/doc/en/contrib/git-commit-messages.html

@sujeshthekkepatt
Copy link
Contributor Author

sujeshthekkepatt commented Dec 2, 2019

@dhmlau yeah got that. I was hoping to do a squash after the code review. I will update accordingly. @dhmlau done that .

The current hasone error message is not appropriate one.
So adds a better message.
@sujeshthekkepatt
Copy link
Contributor Author

@bajtos any updates?

Copy link
Member

@dhmlau dhmlau left a comment

Choose a reason for hiding this comment

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

@sujeshthekkepatt, thanks for your PR. LGTM. I'd like to get at least one more reviews from the maintainers before landing.
@strongloop/loopback-maintainers, could you please review? Thanks!

Copy link
Contributor

@agnes512 agnes512 left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@dhmlau
Copy link
Member

dhmlau commented Dec 16, 2019

@agnes512, thanks for reviewing.
I'm landing this PR because I saw @bajtos' comments have been addressed.

@dhmlau dhmlau merged commit 6a7fc52 into strongloop:master Dec 16, 2019
@dhmlau
Copy link
Member

dhmlau commented Dec 16, 2019

@sujeshthekkepatt, thanks for your contribution. Your PR has landed! 馃帀

@sujeshthekkepatt sujeshthekkepatt deleted the fix/hasone-relation-error-message branch December 16, 2019 03:52
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.

None yet

5 participants