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

generalized the link fixer cog and added more domains #113

Merged
merged 2 commits into from
May 7, 2024

Conversation

ingenium21
Copy link
Collaborator

renamed the class and file name to be more general.

added a dictionary to keep track of whatever domains need to be added or removed.

rewrote methods so that they don't just focus on twitter but the domains in the dict.

cogs/link_fixer.py Outdated Show resolved Hide resolved
Copy link
Contributor

@reddit-tennis reddit-tennis left a comment

Choose a reason for hiding this comment

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

Looks fine, just 1 small nitpick comment

Also just a thought, do we want to strip long URLs with referrer information etc

Copy link
Collaborator

@ash663 ash663 left a comment

Choose a reason for hiding this comment

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

left a comment

@AndyReifman
Copy link
Collaborator

Looks fine, just 1 small nitpick comment

Also just a thought, do we want to strip long URLs with referrer information etc

Yes, we definitely should strip tracking information

updating comment to resolve vaanir's er...comment.
cogs/link_fixer.py Show resolved Hide resolved
cogs/link_fixer.py Show resolved Hide resolved
@ingenium21 ingenium21 merged commit 5f6563a into CommunityToolBox:main May 7, 2024
@ingenium21 ingenium21 deleted the linkFixer branch May 13, 2024 18:38
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.

4 participants