-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Call improvements #544
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
Call improvements #544
Conversation
…ve been interacted with
…gs like additional whitespaces
Yes, when you get a VoIP call at the same time as a real call. Maybe also with call waiting. |
|
if anything , i wish that your pull will be merged AFTER #342 and NOT INSTEAD by @JF002 because it will give a very bad vibe to young dev / student in cpp . Because they would not be able to prove their ability in opensource project and they will be very reluctant to give pull if the pull is retreved/renamed by other (when for example if they are in exam period). Otherwise why did not you have proposed as review the easiest fix ? i had to say it because with @JF002 doing nearly the same with another pull (where the person didn't say anything at least officially in their pull even if it's a very old pull), and you doing it on a pull that should be merged , i felt the project was going in a very bad path , if it were to become accepted here as normal , to take other pull and fix it (and close the original pull) instead of helping it grow by review and merging it. |
I based this PR on his, so all of his commits and work exist here and with his name. I did review his PR but it has been a long time since he worked on that PR. |
|
he was technically in exam period when @JF002 said he was to put it in 1.3 and even jf002 said him in discord to finish his exam before fix the pull , that jf002 can wait that the pull to be merged. so it understandable that he was not working on it. after some cool down , @Raupinger should be able to fix the pull . It would this kind of situation that would make other people reluctant to give pull afterward. |
|
This is nothing more than a PR on top of a PR, that is quite literally how Git (and FOSS actually) works and it's how it should work. There are no authorship issues and both PRs can be closed at the same time. There's no functional difference between having a PR merged and then making a new PR that changes the files or creating a single PR that contains commits from other PR. |
|
true @Avamander that functional difference is none , but a student/young dev need to prove that his code is not garbage unfit to be merged and gain/have some self esteem in his/her ability. if his hard work , is rejected/not considered but other people take his commit (even if in comment they give the proper name) then when student/young dev show their Github account /commit to other people (for example a recruiter) these will say/show that he/she never managed to make code on big functionality that was merged and thus that they worth very few if not nothing.... because github only remember the link to pull not the comment name when in the pull of other when there is a merge. |
Incorrect. Rest of the text that was built on that assumption is also incorrect. |
the assumption may be bad @Avamander but since i will stop , talking about it because originally i wanted to only voice a warning of a situation that worrying me . now the talk can return on the pull content itself. |
|
Thanks @Riksu9000 for helping on that topic! Indeed, @Raupinger told us he was doing his exams and could not work on the PR right now. That's why I didn't merge it in 1.3 and move the PR in 1.4, hoping he would be able to finish the PR for the release (which is not planned yet). I think @Riksu9000 is doing a great work by extending @Raupinger works (while keeping his commits) instead of just creating a new branch/PR. If we decide to merge this branch, @Raupinger work will be merged in the project. See the commit history of this branch: Now, for unknown reasons, sometimes Github does not automatically mark PRs as merged when they are merged by another PR or manually using CLI. If this happen for this PR, and if an eventual recruiter only look at the status of the PR instead of looking at the commit history, the "recruiter" will think @Raupinger did not succeed in adding his code in the project. But... would I take such recruiter seriously? I don't think so! And unfortunately, Github is not open source so we cannot even try to fix that issue.
I heard your voice and I'll make sure the work of all contributors will be visible in the commit history following the state of the art of Git ;) |
thanks @JF002 , please do so for other pulls as well . the other warning/(voicing?) was that it's good to improving another pull as long as : Otherwise young dev will hardly improve themselves.("the forging tales effect") by the way , |
|
Huh, that's quite the wall of text you're treating me here. Since there was some talk about my situation let my weigh in. I don't do any of this because I'm trying to build a resume. I decided to contribute to Infinitime because I enjoy building things and I've wanted to take part in open source for quite a while. This is the first project where it seemed like I could make an impact (KDEs stuff confused the heck out of me for example). If you take a look at my GitHub you'll find plenty of unfinished projects that are the result of a young person biting of way more than they could chew. I've decided to leave all of that public because for me, this isn't about cosmetics, its about me enjoying to learn new things. (And I still learned a lot from those failed projects). Even with all that said, I still have the GitHub link in my CV and will point out things like Infinitime, but there are already some PRs authored by me that have been merged and I intend to continue working on the ones that are still open as well as new ones. a single PR more or less won't make a difference to a sane recruiter. My exams are over btw. and I'm exited to get back to writing code in the next few days. PS.: it's not @JF002's job as a maintainer to help me or other people look good on their CVs |
|
It would probably still be smart for me to review those new changes so we avoid bugs from @Riksu9000 misinterpreting the code I wrote. |
Avamander
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice formatting-wise.
Have you taken a look at clang-tidy warnings, didn't create any new ones?
|
I assume clangd shows all clang-tidy warnings, so I don't think I created new ones. |
I just realized there might be another separate issue here as well. If there are two call notifications at the same time, and we press end call, does the correct call get ended? It doesn't look like the notification InfiniTime sends contains information about which call was ended, so what will happen? |

I've made improvements upon @Raupinger s #342.
The notification preview can now be closed by swiping up. Maybe the notification should pop up from the bottom and be closed by swiping down?
By creating two call notifications at the same time with GB debug, the second call might stop the ringing. Can this actually happen in reality?