-
Notifications
You must be signed in to change notification settings - Fork 254
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
Config file error message for duplicate keys fixed and improved #1547
Config file error message for duplicate keys fixed and improved #1547
Conversation
Added try catch for config error handling as well as a start of a unit test
@amart241 Can you rework the PR title to better indicate what it actually fixes? Maybe something like "Improve config file invalid syntax error feedback"? |
@amart241 Also, please set the project fields for the PR per the pre-approval checklist. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed files according to feedback.
Swore I made this change already, committing again Co-authored-by: Alden Hilton <[email protected]>
I like this suggestion and that is why I used write-warning to begin with it wasn't printing out. Co-authored-by: Alden Hilton <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done with new comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest updates address previous comments. Final test has the error coming thru looking clean and readable by the user. Well done.
@nanda-katikaneni This PR is ready for merge. |
🗣 Description
💭 Motivation and context
This will fix add an error message when 2 or more keys are added to a yaml config file. This will resolve #1218.
Closes #1218
🧪 Testing
I have ran this update with several config files to make sure it works as well as a unit test.
✅ Pre-approval checklist
✅ Pre-merge checklist
PR passed smoke test check.
Feature branch has been rebased against changes from parent branch, as needed
Use
Rebase branch
button below or use this reference to rebase from the command line.Resolved all merge conflicts on branch
Notified merge coordinator that PR is ready for merge via comment mention
Demonstrate changes to the team for questions and comments.
(Note: Only required for issues of size
Medium
or larger)✅ Post-merge checklist