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

Fix: Do not leak codefix IDs in API #1775

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Dedsec0098
Copy link

@Dedsec0098 Dedsec0098 commented Feb 9, 2025

Fix #1752

  • Added uuid instead of using id

  • Imported uuid in the model.py

  • Added test_codefix_id under tests section

Signed-off-by: Shrish Mishra shrish409@gmail.com

@Rishi-source
Copy link

Rishi-source commented Feb 10, 2025

Hey @Dedsec0098,

Great to see your enthusiasm for contributing! It’s awesome that you’re actively working on PRs. Just wanted to share a few tips that might help make your contributions even more impactful:
Quality over quantity – I totally get the excitement of making multiple PRs early on (I did the same when I started!), but one thing I learned from @pombredanne is that well-tested, high-quality contributions matter way more than just numbers. Taking a little extra time to refine and test your code can make a big difference.
Test locally before submitting – Setting up the server on your local machine and testing your changes thoroughly can save a lot of back and forth for maintainers. Also, sharing your test results in your PR description can really help reviewers understand the impact of your changes.
Mark PRs as Draft if they’re not ready – Another great tip from @pombredanne is to mark your PR as a draft if you know it’s still a work in progress.You can also specify the work left in your PR description as well. This helps maintainers quickly see which PRs are ready for review and which still need some work, saving everyone a lot of time.

Now, moving on to some specific review points for this PR…

@Dedsec0098
Copy link
Author

Hi @Rishi-source thanks a lot for sharing me your views, I totally get your point and would definitely start putting test results in my PRs.

@TG1999
Copy link
Contributor

TG1999 commented Feb 13, 2025

@Dedsec0098 please add tests, rest it looks good. Thanks!

@Dedsec0098
Copy link
Author

Sure @TG1999 , I'll start working on adding tests

@Dedsec0098
Copy link
Author

Hey @TG1999 I have added tests to my PR, please let me know if I have done it correctly or some changes are needed.

@@ -0,0 +1,46 @@
import re
Copy link
Contributor

Choose a reason for hiding this comment

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

Add license headers

Copy link
Author

Choose a reason for hiding this comment

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

Hey @TG1999 I have made the changes to add headers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Tests are failing pls check!

Copy link
Author

Choose a reason for hiding this comment

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

Hey @TG1999 I am finding it difficult to solve these tests issues, but i am trying to solve them as soon as possible.

Signed-off-by: Shrish0098 <shrish409@gmail.com>
Signed-off-by: Shrish0098 <shrish409@gmail.com>
Signed-off-by: Shrish0098 <shrish409@gmail.com>
@Dedsec0098 Dedsec0098 force-pushed the Do-not-leak-codefix-IDs-in-API branch from 5762915 to ab7f19d Compare February 21, 2025 08:37
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.

Do not leak codefix IDs in API
3 participants