Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Add component name to property validation error message #275

Conversation

ConorGriffin37
Copy link
Member

@ConorGriffin37 ConorGriffin37 commented Jun 11, 2020

Add componentName to property validation error message. This would be helpful in cases where element tracing is not turned on or provides an unhelpful error message.

Checklist before submitting:

  • Added entry to CHANGELOG.md
  • Added/updated relevant tests
  • Added/updated documentation

@coveralls
Copy link

coveralls commented Jun 11, 2020

Coverage Status

Coverage increased (+0.004%) to 94.188% when pulling 337e0a4 on ConorGriffin37:addComponentNameToPropertyValidationError into 10e7060 on Roblox:master.

Copy link
Contributor

@ZoteTheMighty ZoteTheMighty left a comment

Choose a reason for hiding this comment

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

Can we add a test for this? Might just be able to modify this test:

it("should throw if validateProps returns false", function()

Probably would need to replace the expect...to.throw with a pcall and do some assertions on the error message.

Copy link
Contributor

@ZoteTheMighty ZoteTheMighty left a comment

Choose a reason for hiding this comment

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

This looks good to me! Any objections, @LPGhatguy ?

@@ -188,7 +188,8 @@ return function()
end)

expect(success).to.equal(false)
expect(error:find("MyComponent")).to.be.ok()
local startIndex = error:find("MyComponent")
expect(startIndex).to.be.ok()
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, good catch here!

@ConorGriffin37
Copy link
Member Author

ConorGriffin37 commented Jun 12, 2020

@cliffchapmanrbx Looks like this CLA check doesn't work with forks, not sure if we need to change it?

Edit: Okay, so it just requires a comment to be made before it makes the comment.

Should I just add myself to the whitelist or respond to the bot?

@github-actions
Copy link

CLA Signature Action:

Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you read and sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just by adding a comment to this pull request with this exact sentence:

I have read the CLA Document and I hereby sign the CLA

By commenting with the above message you are agreeing to the terms of the CLA. Your account will be recorded as agreeing to our CLA so you don't need to sign it again for future contributions to our company's repositories.

0 out of 1 committers have signed the CLA.
@ConorGriffin37

@cliffchapmanrbx
Copy link
Contributor

Minor deployment bugs on the bot that I fixed late yesterday, it should work normally now. As it says at http://go/clabot FTEs should not sign the CLA, they should be whitelisted.

Copy link
Contributor

@LPGhatguy LPGhatguy left a comment

Choose a reason for hiding this comment

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

Looks good to me after a small changelog suggestion.

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Lucien Greathouse <[email protected]>
@LPGhatguy LPGhatguy merged commit 60197ac into Roblox:master Jun 15, 2020
@ConorGriffin37 ConorGriffin37 deleted the addComponentNameToPropertyValidationError branch June 15, 2020 17:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants