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: Algolia optimization #13

Merged
merged 1 commit into from
Jan 18, 2024
Merged

Fix: Algolia optimization #13

merged 1 commit into from
Jan 18, 2024

Conversation

Wolmin
Copy link

@Wolmin Wolmin commented Jan 17, 2024

Description and Related Issue(s)

This PR adjusts calls for algolia proxy, to optimize resources usage

Checklist for PR author

  • I have tested these changes locally.

@Wolmin Wolmin added the enhancement New feature or request label Jan 17, 2024
@Wolmin Wolmin self-assigned this Jan 17, 2024
@Esanim
Copy link

Esanim commented Jan 17, 2024

You prob. need to update snapshots for tests to pass.

Copy link

@Hugoo Hugoo 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 the props are confusing

You used to use address as a string, now address is an object with props you will not use at all like is_verified and so on.

I suggest to keep this as lean and clean as possible by keeping the getUniversalProfile() function as simple as possible.

I suggest to:

  1. do an early return: if address.is_contract -> don't even call that function. No need to pass all the mess down the other functions
  2. Or if 1. doesnt not work, to add a "isContract" arg for the getUniversalProfile - so it is clear this things only need a has (address) and a bool and you don't move all these usesless props all around which makes things confusing imo

@Hugoo
Copy link

Hugoo commented Jan 17, 2024

smtgh like that

image

@Wolmin Wolmin force-pushed the fix/algolia-optimization branch from 6d35271 to 0dfc045 Compare January 17, 2024 14:51
@Wolmin Wolmin requested a review from Hugoo January 17, 2024 15:26
@Wolmin Wolmin merged commit f94184d into lukso Jan 18, 2024
7 of 10 checks passed
@Wolmin Wolmin deleted the fix/algolia-optimization branch January 18, 2024 14:58
Wolmin added a commit that referenced this pull request Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

3 participants