Skip to content

Conversation

@plon-Susk7
Copy link
Contributor

  1. In order to generate unit tests, the config file needs to have path where the class is located.
  2. We are using pytest as our framework and the same is passed to our prompt.

@CLAassistant
Copy link

CLAassistant commented Nov 2, 2024

CLA assistant check
All committers have signed the CLA.

@plon-Susk7
Copy link
Contributor Author

Here's an example of PR

plon-Susk7#3

@codelion codelion requested a review from CTY-git November 7, 2024 09:22
Copy link
Contributor

@CTY-git CTY-git left a comment

Choose a reason for hiding this comment

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

I think we should use ExtractModelResponse step to handle extra texts from the LLM.

@plon-Susk7
Copy link
Contributor Author

I think we should use ExtractModelResponse step to handle extra texts from the LLM.

hi @CTY-git , the issues were resolved by your suggestions. I have added extra params to yml file and also mentioned it in README. Let me know if it's enough else I'll try to make changes to ExtractModelResponse.

Copy link
Contributor

@CTY-git CTY-git left a comment

Choose a reason for hiding this comment

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

Could you add a CI test case to generate unit tests on the files in tests/cicd/generate_docstring?

@plon-Susk7
Copy link
Contributor Author

Could you add a CI test case to generate unit tests on the files in tests/cicd/generate_docstring?

Where should I write this test case?

@CTY-git
Copy link
Contributor

CTY-git commented Nov 8, 2024

@plon-Susk7 I am refering to adding a test on github action like this particular step:
https://github.com/patched-codes/patchwork/blob/main/.github/workflows/test.yml#L196

Copy link
Contributor

@CTY-git CTY-git left a comment

Choose a reason for hiding this comment

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

LGTM

@CTY-git CTY-git merged commit 981d5a9 into patched-codes:main Nov 11, 2024
@plon-Susk7
Copy link
Contributor Author

Thanks @CTY-git. Really sorry for naive mistakes that I overlooked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants