-
-
Notifications
You must be signed in to change notification settings - Fork 293
feat(starr-french): add Audio Description (AD) CF #2569
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
base: master
Are you sure you want to change the base?
Conversation
Reviewer's GuideAdds a new "WiTH AD" (Audio Description) custom format for both Radarr and Sonarr, wires it into the French setup guides (EN/FR), and introduces shared description and JSON definition files to support it. Flow diagram for WiTH AD Audio Description CF documentation wiringflowchart TD
subgraph CF_Definition
AD_Desc[with-ad_md\nincludes/cf-descriptions/with-ad.md]
AD_Radarr_JSON[with-ad_json\ndocs/json/radarr/cf/with-ad.json]
AD_Sonarr_JSON[with-ad_json\ndocs/json/sonarr/cf/with-ad.json]
end
subgraph CF_Collections
Radarr_CF_Page[Radarr_collection_of_custom_formats_md]
Sonarr_CF_Page[Sonarr_collection_of_custom_formats_md]
end
subgraph French_Guides
Radarr_FR_EN[radarr-setup-quality-profiles-french-en_md]
Radarr_FR_FR[radarr-setup-quality-profiles-french-fr_md]
Sonarr_FR_EN[sonarr-setup-quality-profiles-french-en_md]
Sonarr_FR_FR[sonarr-setup-quality-profiles-french-fr_md]
Radarr_AD_EN[radarr-french-audio-description-en_md]
Radarr_AD_FR[radarr-french-audio-description-fr_md]
Sonarr_AD_EN[sonarr-french-audio-description-en_md]
Sonarr_AD_FR[sonarr-french-audio-description-fr_md]
end
AD_Desc --> Radarr_CF_Page
AD_Radarr_JSON --> Radarr_CF_Page
AD_Desc --> Sonarr_CF_Page
AD_Sonarr_JSON --> Sonarr_CF_Page
Radarr_FR_EN --> Radarr_AD_EN
Radarr_FR_FR --> Radarr_AD_FR
Sonarr_FR_EN --> Sonarr_AD_EN
Sonarr_FR_FR --> Sonarr_AD_FR
Radarr_AD_EN --> Radarr_CF_Page
Radarr_AD_FR --> Radarr_CF_Page
Sonarr_AD_EN --> Sonarr_CF_Page
Sonarr_AD_FR --> Sonarr_CF_Page
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@NiceTSY your pull request title "Feat (starr french): add Audio Description (AD) CF" does not conform to our naming conventions. Please update the title to match the pattern: "feat|build|chore|style|fix|update|ci(<area>): <description> You can check your title at this regex101 link." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- In the new English audio description includes, there are a couple of small phrasing issues that could be cleaned up for clarity, e.g. change “which is way for visually impaired people to enjoy their media” to “which is a way for visually impaired people to enjoy their media” and “and not be confused with advertisements as they are not” to something like “and should not be confused with advertisements.”
- In the French audio description includes, there are some minor grammar/spelling issues worth fixing: e.g. “ceci ne doit pas être confondus” should agree in number (“confondue/confondues”) and “possèdant” should be “possédant.”
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the new English audio description includes, there are a couple of small phrasing issues that could be cleaned up for clarity, e.g. change “which is way for visually impaired people to enjoy their media” to “which is a way for visually impaired people to enjoy their media” and “and not be confused with advertisements as they are not” to something like “and should not be confused with advertisements.”
- In the French audio description includes, there are some minor grammar/spelling issues worth fixing: e.g. “ceci ne doit pas être confondus” should agree in number (“confondue/confondues”) and “possèdant” should be “possédant.”
## Individual Comments
### Comment 1
<location> `includes/french-guide/radarr-french-audio-description-en.md:4` </location>
<code_context>
+<!-- markdownlint-disable MD041-->
+??? abstract "Audio Description - [Click to show/hide]"
+
+ Some releases are now available with Audio Description, which is way for visually impaired people to enjoy their media.
+ Those are flagged `WiTH.AD` or just `AD`, and not be confused with advertisements as they are not.
+
</code_context>
<issue_to_address>
**issue (typo):** Fix grammar in 'which is way' phrase.
Change this to: `which is a way for visually impaired people to enjoy their media.` to fix the grammar.
```suggestion
Some releases are now available with Audio Description, which is a way for visually impaired people to enjoy their media.
```
</issue_to_address>
### Comment 2
<location> `includes/french-guide/radarr-french-audio-description-en.md:7` </location>
<code_context>
+ Some releases are now available with Audio Description, which is way for visually impaired people to enjoy their media.
+ Those are flagged `WiTH.AD` or just `AD`, and not be confused with advertisements as they are not.
+
+ If you want to prefer those for any reasons, please add the following CF with a score of `101`. You can still use it for information if you decide to not score it.
+
+ | Custom Format | Trash ID |
</code_context>
<issue_to_address>
**suggestion (typo):** Adjust 'for any reasons' to standard phrasing.
Change 'for any reasons' to 'for any reason' for more natural phrasing.
```suggestion
If you want to prefer those for any reason, please add the following CF with a score of `101`. You can still use it for information if you decide to not score it.
```
</issue_to_address>
### Comment 3
<location> `includes/french-guide/sonarr-french-audio-description-en.md:4` </location>
<code_context>
+<!-- markdownlint-disable MD041-->
+??? abstract "Audio Description - [Click to show/hide]"
+
+ Some releases are now available with Audio Description, which is way for visually impaired people to enjoy their media.
+ Those are flagged `WiTH.AD` or just `AD`, and not be confused with advertisements as they are not.
+
</code_context>
<issue_to_address>
**issue (typo):** Fix grammar in 'which is way' phrase (Sonarr EN).
Change the phrase to: "which is a way for visually impaired people to enjoy their media."
```suggestion
Some releases are now available with Audio Description, which is a way for visually impaired people to enjoy their media.
```
</issue_to_address>
### Comment 4
<location> `includes/french-guide/sonarr-french-audio-description-en.md:7` </location>
<code_context>
+ Some releases are now available with Audio Description, which is way for visually impaired people to enjoy their media.
+ Those are flagged `WiTH.AD` or just `AD`, and not be confused with advertisements as they are not.
+
+ If you want to prefer those for any reasons, please add the following CF with a score of `101`. You can still use it for information if you decide to not score it.
+
+ | Custom Format | Trash ID |
</code_context>
<issue_to_address>
**suggestion (typo):** Adjust 'for any reasons' to standard phrasing (Sonarr EN).
Here too, please use 'for any reason' instead of 'for any reasons' for more natural phrasing.
```suggestion
If you want to prefer those for any reason, please add the following CF with a score of `101`. You can still use it for information if you decide to not score it.
```
</issue_to_address>
### Comment 5
<location> `includes/french-guide/radarr-french-audio-description-fr.md:5` </location>
<code_context>
+??? abstract "Audio Description - [Click to show/hide]"
+
+ Certaines releases sont maintenant disponibles avec un canal d'Audio Description, ceci afin de rendre un média accessible aux personnes aveugles ou malvoyantes.
+ Celles-ci sont marquées avec `WiTH.AD` ou `AD`, ceci ne doit pas être confondus avec un média possèdant des pubs (advertisements ou ads en anglais).
+
+ Si vous souhaitez préférer ce type de releases, merci d'ajouter le CF suivant avec un score de `101`. Vous pouvez tout aussi l'utiliser mais à titre informatif sans le scorer.
</code_context>
<issue_to_address>
**issue (typo):** Fix agreement and spelling in French sentence.
Two minor fixes: (1) agreement: either "Ceci ne doit pas être confondu" or better match the plural subject with "Celles-ci ne doivent pas être confondues avec…"; (2) spelling: "possèdant" → "possédant".
```suggestion
Celles-ci sont marquées avec `WiTH.AD` ou `AD`, celles-ci ne doivent pas être confondues avec un média possédant des pubs (advertisements ou ads en anglais).
```
</issue_to_address>
### Comment 6
<location> `includes/french-guide/sonarr-french-audio-description-fr.md:5` </location>
<code_context>
+??? abstract "Audio Description - [Click to show/hide]"
+
+ Certaines releases sont maintenant disponibles avec un canal d'Audio Description, ceci afin de rendre un média accessible aux personnes aveugles ou malvoyantes.
+ Celles-ci sont marquées avec `WiTH.AD` ou `AD`, ceci ne doit pas être confondus avec un média possèdant des pubs (advertisements ou ads en anglais).
+
+ Si vous souhaitez préférer ce type de releases, merci d'ajouter le CF suivant avec un score de `101`. Vous pouvez tout aussi l'utiliser mais à titre informatif sans le scorer.
</code_context>
<issue_to_address>
**issue (typo):** Correct French agreement and accent in Sonarr FR sentence.
As in the Radarr FR file, please update to: `Celles-ci ne doivent pas être confondues avec un média possédant des pubs…` to fix both agreement (`confondus` → `confondues`) and spelling (`possèdant` → `possédant`).
```suggestion
Celles-ci sont marquées avec `WiTH.AD` ou `AD`, ceci ne doivent pas être confondues avec un média possédant des pubs (advertisements ou ads en anglais).
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Deploying with ⚡ Cloudflare Pages
|
|
Deploying with ⚡ Cloudflare Pages
|
| @@ -0,0 +1,17 @@ | |||
| { | |||
| "trash_id": "44ccbcbc74506f208973e1463b11705f", | |||
| "trash_regex": "https://regex101.com/r/GCIcFI", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s just missing the ignore-case flag in the trash_regex. That's not very importante ahah (MULTi can also be MULTI)
Pull Request
Purpose
Add Audio Description CFs to the guide and implement them into the French Guide.
Approach
Open Questions and Pre-Merge TODOs
Requirements
Summary by Sourcery
Add a new Audio Description custom format and integrate it into the Radarr and Sonarr French quality profile guides.
New Features:
Documentation: