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

No longer use url for sourceId in notifications / syncs when not needed #1611

Merged
merged 3 commits into from
Jan 23, 2024

Conversation

WilcoLouwerse
Copy link
Contributor

No description provided.

smisidjan
smisidjan previously approved these changes Jan 23, 2024
Comment on lines 682 to 685
} else {
$normalEndpoint = $synchronization->getEndpoint().'/'.$synchronization->getSourceId();
$endpoint = str_contains('http', $synchronization->getSourceId()) === true ? $synchronization->getEndpoint() : $normalEndpoint;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This can also be a if statement instead of an else.
And try to avoid the if true ? true : false

Copy link
Contributor Author

@WilcoLouwerse WilcoLouwerse Jan 23, 2024

Choose a reason for hiding this comment

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

I still don't like not using else in some cases like this. Because it results in executing code that gets overwritten immediately, but you are right, codacy wants no else statements or 1 line if statements.
will fix

Comment on lines 1355 to 1358
} else {
$normalEndpoint = $synchronization->getEndpoint().($existsInSource ? '/'.$synchronization->getSourceId() : '');
$endpoint = str_contains('http', $synchronization->getSourceId()) === true ? $synchronization->getEndpoint() : $normalEndpoint;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here: This can also be a if statement instead of an else.

@WilcoLouwerse WilcoLouwerse merged commit 27d58c1 into development Jan 23, 2024
3 of 8 checks passed
@WilcoLouwerse WilcoLouwerse deleted the feature/PC108-13/fix-sourceId branch January 23, 2024 11:56
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.

3 participants