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

Add Aliexpress sanitizer #94

Merged
merged 6 commits into from Nov 7, 2022
Merged

Add Aliexpress sanitizer #94

merged 6 commits into from Nov 7, 2022

Conversation

guerda
Copy link
Contributor

@guerda guerda commented Nov 7, 2022

Copy paste from the EbaySanitizer, first version to figure out the inner workings

  • Automated test
  • Resource translations
  • Plugging the sanitizer into ContainerInitializer.kt
  • Testing it really

Closes #91

Copy paste from the EbaySanitizer, first version to figure out the inner workings
@svenjacobs
Copy link
Owner

It seems I need to adjust the GHA workflow for contributions. I use git-secret to encrypt files that are required for deployment to Play Store. However the PGP key is obviously not available in workflow runs of contributors.

@guerda
Copy link
Contributor Author

guerda commented Nov 7, 2022

That's fine, I'll try out my branch first, then give you feedback. It's still draft, so that's OK to fail

Maybe you can add a condition in the workflow if this is a pull request by a contributor for that job?

@svenjacobs svenjacobs added the feature New feature or request label Nov 7, 2022
@guerda guerda mentioned this pull request Nov 7, 2022
@svenjacobs
Copy link
Owner

Please rebase on main. This is already fixed 😉 (#96)

@svenjacobs
Copy link
Owner

There has been at least one ktlint warning. You can check this locally with ./gradlew lintKotlin and fix most warnings automatically with ./gradlew formatKotlin.

@guerda
Copy link
Contributor Author

guerda commented Nov 7, 2022

Will do, I have to try it this evening.

@svenjacobs
Copy link
Owner

Will do, I have to try it this evening.

Sure, no stress 😉

@guerda
Copy link
Contributor Author

guerda commented Nov 7, 2022

The build is fixed. It might already work on paper/CI :)

@guerda
Copy link
Contributor Author

guerda commented Nov 7, 2022

I tested it with the debug version of this CI run and it works smoothly. Product pages are getting cleaned, all other pages do not seem to have tracking params.
Ready to merge from my end.

@guerda guerda marked this pull request as ready for review November 7, 2022 10:28
Copy link
Owner

@svenjacobs svenjacobs left a comment

Choose a reason for hiding this comment

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

LGTM, thank you very much 😃

@svenjacobs svenjacobs merged commit 105bdff into svenjacobs:main Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AliExpress
2 participants