Skip to content

add Kodi JSON-RPC and EventServer services#552

Merged
erig0 merged 1 commit intofirewalld:masterfrom
nl6720:service-kodi
Jun 17, 2022
Merged

add Kodi JSON-RPC and EventServer services#552
erig0 merged 1 commit intofirewalld:masterfrom
nl6720:service-kodi

Conversation

@nl6720
Copy link
Contributor

@nl6720 nl6720 commented Dec 5, 2019

@nl6720 nl6720 changed the title add Kodi JSON-RPC and EventServer add Kodi JSON-RPC and EventServer services Dec 5, 2019
TorontoMedia
TorontoMedia suggested changes Jun 2, 2022
View reviewed changes
Copy link
Collaborator

@TorontoMedia TorontoMedia left a comment

Choose a reason for hiding this comment

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

Thank you for submitting this pull request. Could you please address my change requests and if possible, add the type semantic to the commit message feat(service): add Kodi JSON-RPC and EventServer services

@erig0 erig0 added the blocked Blocked in some way. e.g. waiting on the user that opened the issue or pull request label Jun 2, 2022
@erig0 erig0 removed the blocked Blocked in some way. e.g. waiting on the user that opened the issue or pull request label Jun 17, 2022
erig0
erig0 approved these changes Jun 17, 2022
View reviewed changes
Copy link
Collaborator

@erig0 erig0 left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks @nl6720 and @TorontoMedia !

@erig0 erig0 merged commit e79e99b into firewalld:master Jun 17, 2022
@nl6720 nl6720 deleted the service-kodi branch June 18, 2022 06:41
@marillat
Copy link

marillat commented Jul 24, 2022

Would be nice to know why these files have been added when kodi already
provides these files since kodi 18 released in January 2019 ?
Also kodi install more services thans firewalld
https://github.com/xbmc/xbmc/tree/master/tools/Linux/firewalld-services

Wouldn't it be easier to let applications install firewalld services like systemd does?

@nl6720
Copy link
Contributor Author

nl6720 commented Jul 24, 2022

https://github.com/xbmc/xbmc/tree/master/tools/Linux/firewalld-services

Oh, I never noticed those. Sorry!

This PR should be reverted then.

@nl6720 nl6720 mentioned this pull request Jul 26, 2022
Merged
@nl6720
Copy link
Contributor Author

nl6720 commented Jul 26, 2022

I created #1002 to revert this.

@marillat
Copy link

marillat commented Jul 26, 2022

Thanks

@erig0
Copy link
Collaborator

erig0 commented Jul 27, 2022

Would be nice to know why these files have been added when kodi already provides these files since kodi 18 released in January 2019 ? Also kodi install more services thans firewalld https://github.com/xbmc/xbmc/tree/master/tools/Linux/firewalld-services

Wouldn't it be easier to let applications install firewalld services like systemd does?

I was unaware Kodi provided its own service definitions.

I have no preference for if they're provided by the package or firewalld. If Kodi prefers to ship them, then great!

I will merge the the revert of this, #1002.

This was referenced Aug 26, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erig0 erig0 erig0 approved these changes

+1 more reviewer

@TorontoMedia TorontoMedia TorontoMedia requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nl6720 @marillat @erig0 @TorontoMedia