-
Notifications
You must be signed in to change notification settings - Fork 19
Fix issue #77 #87
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
base: master
Are you sure you want to change the base?
Fix issue #77 #87
Conversation
: Frequent duplicates on default page
aslo4/generator.py
Outdated
| for idx, existing_bundle in enumerate(self.index): | ||
| if existing_bundle.get('bundle_id') == bundle_id: | ||
| # Update the existing entry instead of adding a duplicate | ||
| self.index[idx] = bundle.generate_fingerprint_json(unique_icons=args.unique_icons) | ||
| bundle_exists = True | ||
| break |
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.
You can check if the bundle exists without looping through self.index, looping through just to check for the existence of a bundle id is inefficient.
|
Looking at your change, it's clear you used an LLM, at least don't include certain files like the ones you included as they have nothing to do with your change. Your commit messages can be better too, the things in your opening message should be part of your commit message, see making commits. |
|
I'm horrified. |
|
new to the whole thing ..."horrific" is quite demotivating |
|
Why would we need to improve the efficiency of this? There are only a few activities on the app store, and the code is only ever run by activity maintainers at the point of release. Last I checked, a few times a year. Not horrific, but I am horrified that this has been proposed. Are the tools you are using really that bad as to make this kind of mistake? |
|
Here's the comment text copied for you to use: Thank you for your feedback on the efficiency improvements. I appreciate your perspective on this. You raise a valid point about the frequency of use and scale of the app store. I wasn't approaching this from the perspective of solving an urgent performance problem, but rather as a general code improvement opportunity I noticed while reviewing the codebase. I should mention that I'm very new to open source contribution - I just started contributing 2 days ago. When I saw this pattern in the code, I thought it was something I could easily improve and would be a good first contribution. I was eager to demonstrate my ability to identify and solve problems, even if small ones. The tools I'm using aren't the issue - this was simply a case where I identified a potential improvement in the algorithm (changing from linear search to dictionary lookup) that would make the code more maintainable . The change is relatively small and straightforward, making the code more aligned with Python best practices. I understand your concern about prioritization. Since the current implementation works fine for the current scale, this optimization isn't critical. My intention wasn't to suggest the current code is problematic in practice, but to contribute a small improvement that might be helpful if the number of activities grows in the future. As a newcomer to the project, I'd really appreciate if you could suggest some good first issues that would be more valuable for me to work on. I'm eager to contribute in ways that will have the most positive impact on the project, and your guidance on where to focus my efforts would be extremely helpful. I'm happy to focus on other areas that you feel would bring more immediate value to the project. Could you suggest which aspects of the codebase would benefit most from attention? |
What does that line mean? |
|
I myself didn't understand the last line you wrote so I made use of chatgpt to help me understand it and moreover I framed my answer with its help I think I accidentally copied the whole thing...forgot to delete the initial part.... |
Thank you for contributing, we do want you to contribute and while you can use an LLM, copying and pasting everything from there makes it a bit difficult to take your contribution serious because there's lack of context sometimes and it shows us you didn't even bother taking the time to review or understand the change you copied and pasted. |
|
Yes I understand that completely as a very young learner I tend to get excited and out of excitement I tend to react.could u please guide me as a senior on what to do...where to start |
|
Nothing wrong with being excited, the issue you're trying to fix is one that needs to be fixed. I'll recommend you try to fix the issue, taking into consideration this time the comments made above, and test your change locally and be sure the issue is fixed, then write a good commit message detailing your changes - see making commits. |
(search.js):
Added duplicate checking in the loadAllActivities() function
Created a Set to track bundle_ids that have already been displayed
Added a condition to only display an activity if its bundle_id hasn't been seen before
(generator.py):
Modified the index generation process to check if an activity with the same bundle_id already exists in the index
If a duplicate is found, the code updates the existing entry instead of adding a new one