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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[docstring] Fix docstring for LlamaConfig #26658

Closed
wants to merge 6 commits into from
Closed

[docstring] Fix docstring for LlamaConfig #26658

wants to merge 6 commits into from

Conversation

pavaris-pm
Copy link
Contributor

What does this PR do?

Fixes #26638 by fixing a typo in docstring of LlamaConfig

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@ydshieh
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@abzdel
Copy link
Contributor

abzdel commented Oct 7, 2023

Hey - lmk if you want help fixing CI on this. Looks like it only failed because of a dependency - happy to try to troubleshoot this with you!

@pavaris-pm
Copy link
Contributor Author

@abzdel Hi!, thanks for your help. can you please help me fix this CI? I'm not sure which part make test failed. do you have any idea?

happy to try to troubleshoot this with you too!

@abzdel
Copy link
Contributor

abzdel commented Oct 7, 2023

@pavaris-pm no worries! When I click the failed task I see this:
image

Which makes me think you may have forgotten to run make fixup before you committed the change. I would go back, run make fixup, commit/push the changes (to your branch) again and see if CI passes. CI should automatically re-run on this PR if you do this, but let me know if other bottlenecks occur here.

@pavaris-pm
Copy link
Contributor Author

@abzdel Thanks for you help run make fixup. That's very helpful for me. I'll help you take a look at it.

@abzdel
Copy link
Contributor

abzdel commented Oct 7, 2023

@pavaris-pm it's no problem, let me know if this fixes the issue!

@pavaris-pm pavaris-pm closed this by deleting the head repository Oct 9, 2023
@pavaris-pm
Copy link
Contributor Author

@abzdel i just made a mistake here that cause me to delete the forked repo. i will re-fork repo and do as you recommend. After that, i will mentioned you again with my new PR. Thank you again !

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.

[Community Event] Docstring Sprint
2 participants