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

Fixing agent code. #1516

Merged
merged 18 commits into from
Dec 10, 2021
Merged

Conversation

nharper285
Copy link
Contributor

@nharper285 nharper285 commented Dec 9, 2021

Summary of the Pull Request

What is this about?
Fixing an agent code error related to clippy that occurs during the build.

Original errors:
error1
error2

PR Checklist

  • Applies to work item: #xxx
  • CLA signed. If not, go over here and sign the CLI.
  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Info on Pull Request

What does this include?
Changes to lib.rs

Validation Steps Performed

Confirming build runs.

Co-authored-by: Cheick Keita <kcheick@gmail.com>
nharper285 and others added 2 commits December 9, 2021 13:16
Co-authored-by: Cheick Keita <kcheick@gmail.com>
Copy link
Member

@ranweiler ranweiler left a comment

Choose a reason for hiding this comment

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

  1. What was the actual lint?
  2. Why did you change the program semantics, and why is that change okay / how did you test it?

@chkeita
Copy link
Contributor

chkeita commented Dec 9, 2021

  1. What was the actual lint?
  2. Why did you change the program semantics, and why is that change okay / how did you test it?

@ranweiler i was the one that proposed the fix
here is the lint error
image

i tested it with string with and without ')' in it to see if it has the same behavior

@nharper285
Copy link
Contributor Author

  1. What was the actual lint?
  2. Why did you change the program semantics, and why is that change okay / how did you test it?

Lint errors linked above. It's not clear to

  1. What was the actual lint?
  2. Why did you change the program semantics, and why is that change okay / how did you test it?

@ranweiler i was the one that proposed the fix here is the lint error image

i tested it with string with and without ')' in it to see if it has the same behavior

I added both errors to the original description

@nharper285
Copy link
Contributor Author

nharper285 commented Dec 9, 2021

@ranweiler & @chkeita - I was slowly working through some of the other clippy errors and I suddenly got this dump:
image

This seems like it has a lot to do with coverage data? Is it appropriate to remove all of these unused fields? The last two have to do with 'large size difference between variants'? I can 'box' those, but I'm not sure if its correct for me to remove all those other fields.

@nharper285 nharper285 merged commit 94e2904 into microsoft:main Dec 10, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jan 10, 2022
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.

None yet

4 participants