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.

Sign up for GitHub

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

Jump to bottom

Removal of false flag for empty channels. #14170

Closed
wants to merge 7 commits into from
Closed

Removal of false flag for empty channels. #14170

wants to merge 7 commits into from

Conversation

Tinwarble
Copy link
Contributor

@Tinwarble Tinwarble commented Jul 10, 2018 •
edited

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@nsenica
Copy link
Contributor

nsenica commented Jul 11, 2018

A minor comment: the Log text on line 175 doesn't reflect the if condition above :)

Rechi
Rechi reviewed Jul 13, 2018
View reviewed changes
tools/android/packaging/xbmc/src/channels/SyncProgramsJobService.java.in Outdated
@@ -170,7 +170,7 @@ public class SyncProgramsJobService extends JobService
if (cursor != null && cursor.moveToNext())
{
Channel channel = Channel.fromCursor(cursor);
if (!channel.isBrowsable())
if (channel.isBrowsable())
{
Log.d(TAG, "Channel is not browsable: " + channelId);

This comment was marked as spam.

Sign in to view

@Rechi Rechi added Type: Fix non-breaking change which fixes an issue Platform: Android v18 Leia labels Jul 16, 2018
@Rechi
Copy link
Member

Rechi commented Jul 18, 2018

@Tinwarble the commits have to be squashed.

@Tinwarble
Copy link
Contributor Author

@Rechi no idea what that means? Only commented this change because Koying recommended, beyond that I'm at a loss.

@Rechi
Copy link
Member

Rechi commented Jul 19, 2018

The actual logic code change and the log message fix have to be one commit, because non of them is valid alone.
Search for git squash commits, if you don't know how to do it.

@koying
Copy link
Contributor

koying commented Jul 19, 2018

That won't work. You have to remove the "isBrowsable" test altogether and only keep the "else" block.
@fritsch understood what I meant.

@Tinwarble
Copy link
Contributor Author

@koying which is what I asked on slack. So is this what you meant?
https://github.com/Tinwarble/xbmc/blob/master/tools/android/packaging/xbmc/src/channels/SyncProgramsJobService.java.in

@koying
Copy link
Contributor

koying commented Jul 20, 2018

Yep, that's the idea.

Rechi
Rechi reviewed Jul 20, 2018
View reviewed changes
tools/android/packaging/xbmc/src/channels/SyncProgramsJobService.java.in Outdated
deletePrograms(channelId, medias);
}
else
{
XBMCJsonRPC jsonrpc = new XBMCJsonRPC();

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Rechi
Rechi reviewed Jul 21, 2018
View reviewed changes
tools/android/packaging/xbmc/src/channels/SyncProgramsJobService.java.in
}
XBMCJsonRPC jsonrpc = new XBMCJsonRPC();
if (uri.isEmpty())
{

This comment was marked as spam.

Sign in to view

@koying
Copy link
Contributor

koying commented Jul 21, 2018

Team-Kodi, as (contribution-)friendly as ever...

Rather than bug @Tinwarble , who is not a dev and I'm not sure he wants to be one, it would be faster for any of the devs that actually had a look at this to do it. I'll take him 1 minute, and can probably be done directly from github.

Next, you'll ask him to squash and he will be lost again...
He's your team-mate, for god's sake!

@Hitcher
Copy link
Contributor

Hitcher commented Jul 22, 2018

ping @fritsch

@Rechi
Copy link
Member

Rechi commented Jul 22, 2018

What should @fritsch do? The PR can't be merged in it's current state as it doesn't follow our guidelines.

See https://github.com/xbmc/xbmc/blob/master/docs/CONTRIBUTING.md#pull-request-guidelines and https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#3-indentation

@Hitcher
Copy link
Contributor

Hitcher commented Jul 22, 2018 •
edited

Sorry, I was just trying to help and saw that koying said fritsch understood.

EDIT: Is that all that's wrong now then?

@Rechi
Copy link
Member

Rechi commented Jul 22, 2018

That is already fixed.

@Tinwarble Tinwarble closed this Jul 22, 2018
@Rechi Rechi removed the v18 Leia label Jul 23, 2018
@timfehr timfehr mentioned this pull request Jul 30, 2018
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rechi Rechi Rechi left review comments

Assignees
No one assigned
Labels
Platform: Android Type: Fix non-breaking change which fixes an issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Tinwarble @nsenica @Rechi @koying @Hitcher