-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add a few MMS fixes. #246
Add a few MMS fixes. #246
Conversation
Both the getMMS and getSMS APIs now accept an 'all_messages' parameter to load the other type at the same time.
Setting this eliminates the vertical padding when an image is scaled to fit the conversation width.
Hi @michaelkourlas. I've been using a local build with the above for over a month now without issue. Any feedback on this PR. or MMS support in general? (I'm trying to decide if I want to set up push notifications) |
For copyright and maintenance reasons I prefer to write my own code, so it's unlikely I'll end up merging this PR. I don't have any immediate term plans to finish the MMS work, so if you'd like to add push messaging support for your own use in the meantime, please do! |
Okay, thanks @michaelkourlas. Personally with these changes I'd consider the MMS work 'finished'. I'm not sure what else you had planned, but for me it works a treat. As for the above changes, it's just a couple of simple fixes and I couldn't care less about the copyright or authorship, so feel free to copy/paste them to your own commit. I hereby multi-license this code under WTFPL, CC0, Unlicensed, and MIT-0. |
@KenMacD If @michaelkourlas doesn't want to do the pull, could you release a build? |
@KenMacD you don't need to declare a new license. It is Apache-2.0, which states, in part:
Which is fine because this means you contribution is under the same copyright terms, notably:
So there is no real copyright conflict. I'm not sure that I understand the objection (nor would I want to speculate) given that other contributions have been merged in the past. Presumably the argument about maintainability could be valid if you are introducing a feature which could create a larger support burden or which is too big or complex to be brought under one individuals maintainership. The former would be an argument to adopt plugin functionality with separate issue tracking, while the later would be for co-maintainership, or even a community fork if absolutely necessary. |
@yulman19 To avoid the numerous issues that would cause, I was hoping to avoid creating one. That's partially why I asked for feedback here. If required I could generate one though.
@JohnMertz well, I'm not a lawyer and playing devils advocate, I'm not sure how much weight I'd put in me having implicitly entered in to a contract based on a license I haven't even read let alone signed anything. If I make my repo license state that contributors agree to pay me $1M for every PR opened I doubt I'll make any money. Maybe the conflict is around the relicensing mess is the problem (eg apparently Apache-2.0 isn't compatible with GPL-2.0)? But as I said, I couldn't care less about authorship of these tiny fixes. Apache-2.0, copy/paste, whatever. I just needed mms support to pass the 'kid test', like others in #190. Heck feel free to copy/paste and then put in the changelog that the mms code was delayed because of me if you want. |
Yes, it's not clear to me whether section 5 of the license is actually enforceable in practice. The right way to do things would be to obtain a signed contributor license agreement or copyright assignment, but I don't really want to go through that trouble. With respect to maintenance, I tend to find it easier to maintain code that I wrote. Beyond those two reasons, there's also the simple fact that as this is a hobby project, I prefer to write all of the code myself and have complete control and ownership over it. That's just part of the appeal for me. It's true that I have accepted PRs in the past. However, they were very small, and even so I probably wouldn't accept them today. |
Hopefully not getting too far off topic here, but might I kindly suggest putting that in a CONTRIBUTING.md to warn anyone who might be considering spending time on substantial changes? |
Yep, that's a good idea. I'll put something together. |
Hi |
This seems tragic. Aren't we talking about four lines of code here? Is there any publicly-available apk of this fork? @KenMacD you previously said "If required I could generate [a build]", just wondering if that is still possibly on your to-do list. |
@xenotropic Last month I reluctantly posted a debug build on my fork. It doesn't use the Google APIs for notifications (I don't have the key for that), so you'll have to weigh mms against quicker notifications.(I'd like to add UnifiedPush to it eventually). |
Hi @KenMacD , I have downloaded and installed the debug build. Thank you. I have noticed that I still get an empty line occasionally when there is only a picture attached to the message. Looking forward to the push notifications. Is there also the ability to add an attach file to the message so a user can send MMS picture messages? Cheers. Keep up the awesome work. |
@KenMacD Your debug build is working pretty good for me, thank you. Do you know if the limiting factor for multi-recipient MMS ("group texts") is the VoiP.ms API -- i.e., that the information just isn't available? Or is the information there to compile group messages into threads, but no one has had the time to incorporate it into the app? Separately, you might also consider turning on issues on your fork. |
In testing the
mms
branch I ran in to a few issues that these commits fix:(Re: #190)