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

feat: force user agent #53

Merged
merged 4 commits into from
May 7, 2024
Merged

feat: force user agent #53

merged 4 commits into from
May 7, 2024

Conversation

Benoit382
Copy link
Collaborator

@Benoit382 Benoit382 commented May 1, 2024

What

Force the user agent
Clean of the previous PR #49

Issues

#48

@@ -120,6 +120,7 @@ class Api
* @param CacheInterface|null $cacheInterface
*/
public function __construct(
public readonly string $userAgent,
Copy link
Collaborator Author

@Benoit382 Benoit382 May 1, 2024

Choose a reason for hiding this comment

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

We could add the user agent at the end BUT nobody will use or define it.

@Benoit382 Benoit382 changed the title Update feat: force user agent May 1, 2024
@Benoit382 Benoit382 marked this pull request as ready for review May 1, 2024 12:18
@Benoit382 Benoit382 requested a review from a team as a code owner May 1, 2024 12:18
Copy link

sonarqubecloud bot commented May 1, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@teolemon teolemon merged commit 4fa67f5 into openfoodfacts:develop May 7, 2024
6 checks passed
@epalmans
Copy link
Member

be aware that this will introduce a breaking change. Not sure if forcing the useragent is really that important (no way to verify anyhow)?... if not, maybe revert this - or add as optional parameter at the end of the constructor?

If we keep it in, we'd need to tag it as 0.4.x (at least)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants