Future

[citruspi]

Yeah, so a couple things.

1. I've been an awful maintainer for this project for the last year or so.
2. I am still interested in maintaining the project - I'm not abandoning it.

In the last year or so, I've dropped out of college, gotten a job, moved to a new city, travelled a bunch, etc. It's been a pretty busy year and I haven't been able to give this project the love that it and its users deserve.

In the last two weeks I've also moved into a new place and I'm still getting settled in and sorting everything out.

Having said that, I do plan on restarting active development on this project in the next couple weeks.

I'm not entirely sure of the way going forward though.

There's an awesome pull request - #56 - which fixes a bunch of issues and revamps the UI. However that pull request is for the 0.5.X branch, which is in Objective-C. I started work on a Swift version (in the master branch) prior to the pull request, however that port is not complete. There is now a pull request - #70 - which fixes some build issues in that branch.

To be honest, I probably fucked up re: the port and the branches. I'm open to feedback on "the path" going forward.

Thoughts?

[citruspi]

cc: @sebj @nbgraham

[nbgraham]

I've never used Objective-C and Swift seems to be the way things are moving, so my vote would be to switch completely to Swift. I like all the changes from #56 and I would be happy to implement them in Swift.

[sebj]

Hey, I'd be happy to implement my changes in Swift too 😆

[sebj]

;D But yeah, haven't checked out the Swift side to be honest. If it doesn't build on latest Xcode/Swift, make sure it does, then divide up work on porting 56 or something?

[citruspi]

Sweet, thanks guys.

Would the two of you be interested in becoming collaborators? I'd give you guys push access.

I'd still want pull requests as opposed to directly pushing to the repo just for the sake of code reviews, but I'd trust you guys to merge in code as necessary.

[nbgraham]

@sebj Yeah that makes a lot more sense. Let me know if you want any help with that.

Seems like #56 should be merged into the 0.5.x for it's functionality and closure, but I have no idea what that entails. Not sure I could be any help there.

I thought I got Swift working with #70 (it was locally) but the CI failed

[citruspi]

Sweet, done. You guys should have push access.

Only other thing which I should probably mention - the 0.5.X branch was MIT licensed. The Swift branch is in the public domain (via The Unlicense).

[sebj]

@citruspi Sure – thanks for the invite! 😃 #56 should merge cleanly into 0.5.x (/ @nbgraham ) so that could be done if you wanted to @citruspi. Also could be worth editing the readme on that branch so people know development is master only? I don't know

[citruspi]

I'm not opposed to merging in #56 for closure.

I think that ideally, going forward, the master branch should be considered stable. So perhaps:

1. Merge in #56
2. Rename master to swift-dev or something
3. Rename 0.5.X to master

Would that make sense? Or would it just cause more confusion?

[sebj]

@citruspi Ooh that's a good idea if we can do that imo

[citruspi]

@sebj FWIW I did run your branch when you originally submitted and I believe I noticed a small bug. I can double check when I've got some free time, but I believe that the value of the Notifications checkbox wasn't saved to user preferences so if a user disabled notifications and restarted the application, it would be re-enabled.

[nbgraham]

@citruspi Thanks for the invite! And good luck with the new place and job!

So who wants to do:

1. Merge in #56
2. Rename master to swift-dev
3. Rename 0.5.X to master

[citruspi]

@nbgraham Thanks!

I've also just got a new laptop, so I don't have Xcode installed or setup or anything. I've started downloading it, I just want to confirm that I can build and run #56 and everything works before it gets merged in so that we can create a new release.

Assuming everything works, I can drop a comment in the thread for #56. I'd probably say that whoever merges in #56 can also rename the branches.

However, we can also rename the branches now, prior to merging in #56. I believe that there's an option to edit the branch the pull request is being merged into, so if we rename 0.5.X to master now, #56 can be updated to merge into master instead of 0.5.X.

Any preferences?

[sebj]

@citruspi Can we merge before renaming? Feels easier/safer in my eyes lol. And on that bug, you're right – just recreated it. After we merge I'll commit again with a fix?

[citruspi]

@sebj Okay, I'll hold off on renaming the branches until #56 gets merged in.

Would you be willing to commit the fix to #56 prior to merging it in?

[sebj]

@citruspi Yep sure. Could we.. enact this repo plan from tomorrow? (Sorry!) I'm pretty exhausted and want to sleep at the moment lol. Would rather approach this bright-eyed tomorrow morning (UK time).

[citruspi]

Sure thing. Once you commit the fix, feel free to merge the pull request and rename the branches. I'm on the US East Coast so if you wait for me, it may be a couple hours after your morning.

[sebj]

@citruspi Ok, sounds good!

[nbgraham]

Seems like I don't have anything to do!

Any thoughts on #59?

[citruspi]

@nbgraham I haven't had a chance to take a in-depth look at that pull request, but it looks mostly fine after a cursory glance. I've added a comment about some code which doesn't seem to be used.

I also haven't had a chance to try building and running or confirming that it does indeed fix, but based on the code and the comments in #38 and #69 it looks like it should.

However, it's worth noting that #59 does two things:

1. Fixes #38
2. Creates a method deliverNotification for delivering notifications

56 does, among other things, both of those:

1. Fixes #38
2. Creates a method deliverUserNotification for delivering notifications

So I'm not sure if it's worth merging in #59 and creating merge conflicts for #56 for something which should be fixed by #56 either way.

[RowanKaag]

Just my two cents, I'm really glad this project will be getting some love again. Love this application :)

[sebj]

@citruspi @nbgraham Just merged #56 into 0.5.X, and fixed that bug @citruspi mentioned.

[sebj]

Commented on and closed #27, #18, #23 and #53.

[sebj]

@citruspi Setup and pushed swift-dev branch, which is just a clone of master, but to remove master we need to set the default branch to swift-dev, which can only be done by yourself (repo Settings => Branches => Default Branch). Then renaming 0.5.X to master should be easy.

[citruspi]

@sebj Awesome stuff on #56. I'm also going to close #38, #45, and #62 since they were fixed by #56 as well as #59 since it doesn't add anything new at this point.

[citruspi]

Branches should also be fixed as well now.

master => 0.5.X Objective-C swift-dev => Swift port

[sebj]

@citruspi Great :) How do we approach this next? I'm happy to carry on porting from master to swift-dev

[citruspi]

Good question. :)

So I think there's two action items:

1. Push out a release for the changes in #56. I'd consider that version 0.6.0. Thoughts?
2. Finish the port to Swift.

For the Swift port, I'm wondering how much sense it would make to actually create issues for work and assign them to people or to try using GitHub Projects. I've created a project for the port here.

[sebj]

All sounds good! Seems like you can't tag or mention people in Projects cards, but you can keep track of what's being worked on. Would be cool to use as a basic todo list separate from long-standing issues that exist.

[citruspi]

Yeah, a basic todo list would probably make sense. Perhaps just a new issue with a checklist of items.

re: Tagging or mentioning people in project cards, you can add issues to the project if you hit the "Add cards" button on the top right. So I think we'd create issues, assign, mention, etc. in the issue and then track the issue via the project.

[sebj]

Gotcha, sure thing

[sebj]

@citruspi How would you feel about switching swift-dev and master branches, seeing as swift-dev is on-par with master now?

[citruspi]

@sebj Woah, I didn't realize that the swift-dev branch was on par master, nice work!

When you say switching the branches, do you mean making the contents of swift-dev the "live" code? i.e. merging swift-dev into master?

I just tried building the swift-dev branch but received the error

fatal error: unexpectedly found nil while unwrapping an Optional value
2017-01-01 11:43:24.002537 Spotify Notifications[16558:275596] fatal error: unexpectedly found nil while unwrapping an Optional value

on the line

let currentItemUrlRef: NSURL = LSSharedFileListItemCopyResolvedURL(currentItemRef, 0, nil).takeRetainedValue()

in LaunchAtLogin.swift.

Side note, could we use pull requests when merging new code (including into the development branch (e.g.swift-dev))? So basically feel free to push code directly to feature branches but then submit a PR for the merge from feature to dev. I've got complete faith in you to merge the PRs in if there's no feedback within ~12 hours or so, but it would just help me keep track of new development. I was totally unaware of the work that you'd put into the swift-dev branch until now.

Happy New Year!

[sebj]

@citruspi Happy New Year to you too! 😃

On PRs: I think my New Year's resolution should be to actually use feature branches & PRs more often (sorry!) 😬

I believe I've solved that issue, along with a couple other bugs on the swift-dev branch (let me know, seemed to be fine for me in testing).

As for the future of the branches, I'm unsure as to whether swift-dev can be merged into master cleanly; alternatively, could do a switcheroo renaming master to obj-c or legacy, and swift-dev to master.

Based on the progress in swift-dev (using Swift 3, Xcode 8, successful Travis build), would it be ok to close the existing pull request too?

[citruspi]

I believe I've solved that issue, along with a couple other bugs on the swift-dev branch (let me know, seemed to be fine for me in testing).

I'm stepping out right now but I'll try building it again when I get back.

As for the future of the branches, I'm unsure as to whether swift-dev can be merged into master cleanly; alternatively, could do a switcheroo renaming master to obj-c or legacy, and swift-dev to master.

Gotcha, yeah, a switcheroo makes sense.

Based on the progress in swift-dev (using Swift 3, Xcode 8, successful Travis build), would it be ok to close the existing pull request too?

Yep, go for it.

[citruspi]

@sebj I've pulled down the latest changes and the application now builds but notifications don't appear and the status of Spotify (at the top of the menu) is stuck to Not Playing. I'll look into it when I've got a bit of time. I don't know if it's an application issue or a problem with my system (but the current v0.6.0 Objective-C build runs without an issue so I suspect it's the former).

[sebj]

@citruspi I just made some more changes locally, I think I've fixed that too. 😬

Also may have bundled in changes to prefs: & an about window

[citruspi]

Sweet!

@ me when you push up the changes and I'll take a look.

[sebj]

@citruspi Just pushed!

[citruspi]

Close but no cigar.

The Spotify play state in the menu updates but there still aren't any notifications and manually triggering a notification via the global shortcut results in a notification that there's nothing playing.

Also, sweet About window! 😁

[citruspi]

Just realized that the Last.fm menu item is also disabled.

[sebj]

@citruspi Argh. I think I've just fixed the problem you mentioned with a very small fix (lemme know).

[citruspi]

@sebj, thanks for fixing #81 and taking a look at the changes in #84.

I've been at a wedding for the last couple days, but I'll be back tomorrow and I'll take a look at the state of the swift-dev branch over this weekend. Hopefully we can push it out and switch the branches by next weekend (I'll be traveling from India to Qatar to New York to Boston from next Tuesday to next Friday, so I'll be mostly offline).

For what it's worth, you can probably just close #84. Like you said, some of @JackWa's changes are covered in changes you've made to the swift-dev branch and to be honest, I don't love the idea of adding the GitHub logo to the application.

Thanks again.

[sebj]

@citruspi That's fair enough, sounds good to me. Apologies if I caused you to get spammed with Github notifications(!). I've just been trying to clear some of the open issues on a couple of my Github repos recently, hence some activity here.

I've closed #84, but looks like my tentative comment on #81 has been proved wrong. Otherwise, #67 is covered, but I've left a line commented out that would remove featured artists from the track name if we've pulled them out for the notification subtitle.