Skip to content

issue fix #1129#3878

Merged
Cli4d merged 4 commits intoOpenTermsArchive:mainfrom
PranamBhat:issueFixPranam
Jul 25, 2025
Merged

issue fix #1129#3878
Cli4d merged 4 commits intoOpenTermsArchive:mainfrom
PranamBhat:issueFixPranam

Conversation

@PranamBhat
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Member

@Cli4d Cli4d left a comment

Choose a reason for hiding this comment

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

Hi @PranamBhat,

First of all, great job on making this contribution! I have reviewed your pull request, and I'm requesting some changes. The tracking of these terms is done by scraping the URL of the specific term. This process relies on the JSON file being able to accurately point to the areas of focus while checking the URL, which is mainly achieved using CSS selectors.

I recommend using simple CSS selectors to identify areas of the document. Avoid complex selectors, such as pseudo-classes ([example link to pseudo-classes]), as this will reduce the need for maintenance when changes occur to the URL selectors.

I also suggest reading through the documentation on declaring the service, which explains the components of the JSON file. This should help you modify your JSON file, after which you can request a review from me. If you need any assistance, feel free to mention me in the pull request, and we can work through it together. I feel there's no need to use range selectors (like start after/end after); a simple CSS selector will suffice.

Copy link
Copy Markdown
Member

@Cli4d Cli4d left a comment

Choose a reason for hiding this comment

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

I've updated the selectors for this particular contribution. Selected some simple selectors focused on accurate tracking of the legal content in these terms. It looks good to me ✅ and I will proceed to merge it

Thank you @PranamBhat for your contribution

@Cli4d Cli4d merged commit a941627 into OpenTermsArchive:main Jul 25, 2025
4 checks passed
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.

2 participants