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

Minor Message Discovery Update #39

Merged
merged 3 commits into from
Mar 5, 2023
Merged

Minor Message Discovery Update #39

merged 3 commits into from
Mar 5, 2023

Conversation

Tech-TTGames
Copy link
Owner

Embeds are now fetched while discovering messages that lack content.

@Tech-TTGames Tech-TTGames added the Type: Enhancement An improvement to an already existing feature. label Mar 4, 2023
@Tech-TTGames Tech-TTGames requested a review from kk5dire March 4, 2023 10:15
@Tech-TTGames Tech-TTGames enabled auto-merge March 4, 2023 10:15
@Tech-TTGames Tech-TTGames requested a review from benfoster04 March 4, 2023 15:50
Copy link
Collaborator

@benfoster04 benfoster04 left a comment

Choose a reason for hiding this comment

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

I would suggest that the variable 'data' is initialised prior to the if/else statements due to its use after. Could that variable also be renamed for clarity? I'm happy otherwise, just a couple of nit picks.

@Tech-TTGames Tech-TTGames requested a review from benfoster04 March 5, 2023 15:40
@Tech-TTGames
Copy link
Owner Author

I would suggest that the variable 'data' is initialised prior to the if/else statements due to its use after. Could that variable also be renamed for clarity? I'm happy otherwise, just a couple of nit picks.

I see no practical reason to initialize the variable before considering we have it initialized in the else. Will commit name change

Copy link
Collaborator

@benfoster04 benfoster04 left a comment

Choose a reason for hiding this comment

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

LGTM

@Tech-TTGames Tech-TTGames merged commit 9005bf1 into main Mar 5, 2023
@Tech-TTGames Tech-TTGames deleted the dev-minor branch March 5, 2023 17:23
@Tech-TTGames Tech-TTGames added Status: Completed It's done! The feature/fix has been deployed. Archive: Old Style This issue has been submitted before a major reform in issue style. It cannot be used as reference. labels Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Archive: Old Style This issue has been submitted before a major reform in issue style. It cannot be used as reference. Status: Completed It's done! The feature/fix has been deployed. Type: Enhancement An improvement to an already existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants