Skip to content

Conversation

@Pragati5-DEBUG
Copy link

(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

: Frequent duplicates on default page
Comment on lines 848 to 853
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
Copy link
Member

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.

@chimosky
Copy link
Member

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.

@quozl
Copy link
Contributor

quozl commented Mar 14, 2025

I'm horrified.

@Pragati5-DEBUG
Copy link
Author

new to the whole thing ..."horrific" is quite demotivating

@quozl
Copy link
Contributor

quozl commented Mar 14, 2025

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?

@Pragati5-DEBUG
Copy link
Author

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?

@quozl
Copy link
Contributor

quozl commented Mar 14, 2025

Here's the comment text copied for you to use:

What does that line mean?

@Pragati5-DEBUG
Copy link
Author

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....
Today was really hectic my braincells are out of fuel now.....

@chimosky
Copy link
Member

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.... Today was really hectic my braincells are out of fuel now.....

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.

@Pragati5-DEBUG
Copy link
Author

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

@chimosky
Copy link
Member

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.

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