Skip to content

Conversation

FanisBak
Copy link

@FanisBak FanisBak commented Jun 7, 2024

Created a new file for suspend mode named suspend.py with changes to the code that I believe snow better and more correct. I removed the suspend mode from tools.py and I made minor changes to the app.py.

@buhtz
Copy link
Member

buhtz commented Jun 7, 2024

Why did you opened a new PR? You can work on your previous PR and just add new commits to it.

@FanisBak
Copy link
Author

FanisBak commented Jun 7, 2024

I made the most things the we talked about in the previous pull request. I created a new file named suspend.py with the suspend mode feature and removed it from tools.py file. Also made minor changes to the app.py file so as to recognise the new file (suspend.py). Unfortunately I see that it has changed some (spaces) again in the app.py file, sorry about that but I don't think that will create any problems.
Also @buhtz you remember correct. I am a student and I have an assignment ,to make my first contribution on an OSS project. The assignment is due 12/06/2024.

@FanisBak
Copy link
Author

FanisBak commented Jun 8, 2024

Why did you opened a new PR? You can work on your previous PR and just add new commits to it.

I thought you preferred a new pull request containing the new file.

@buhtz
Copy link
Member

buhtz commented Jun 8, 2024

Dear Fanis,
I really appreciate your effort to contribute. But I don't see that we can come to end here with this specific issue. I see some communication problems because you have not answered some of my initial questions in #1752. And because I do this in my spare time I don't want to waste more resources on this with asking the same questions again.

Beside of that that PR definitely won't get merged on time until 12th July 2024. There is to much work to do.

Please keep in mind that a pull request is a delicate matter. We assuming 10th of thousands of users we have and who trust there backup data to that piece of software. We take this responsibility serious. That is why I ask some details about this PR you might not find very important. Also such "tiny" problems with indention are not so tiny as you might think.

So please tell me if and how you want to proceed?

Kind regards,
Christian

@buhtz buhtz added Feedback needs user response, may be closed after timeout without a response PR: Modifications requested Maintenance team requested modifications and waiting for their implementation labels Jun 8, 2024
@buhtz buhtz marked this pull request as draft June 8, 2024 08:57
@FanisBak
Copy link
Author

FanisBak commented Jun 8, 2024

Mister Christian,
I understand that there is much work to do checking the PR.
Also the indention sure is a problem, I do not take that lightly and I really apologise about that, this should not have happened.

To answer your questions, I do not have any experience with the dbus so I do not know what could change so I left it the same. I have no solution for that. All I have done about this is to change the name from 'DBUS_SHUTDOW' to 'DBUS_SLEEP'. I did not answer the first time hoping I will find something.

I created a new branch as you asked, named 'suspendmode'.
I also checked if my fork is outdated and it is not.

In this PR I removed all duplicate methods and I used the already existing ones.
Let me explain now what the code I wrote does:

In the function 'sleep' the main think is that firstly checks if backintime process runs ,with the 'processExists' function.
if it does we use the 'pidsWithName' functions to get a list with the PID's of backintime process and store it in a variable named 'pids'. Then we use this with subprocess to suspend the process of the app using the 'kill -SIGSTOP' command with the first pid of 'pids' list. That is the main operation of my code. There should be also added a wake function to start the app again. I found that this would happen with the same way as the suspend but if we used 'kill -SIGCONT' command in the place of 'kill -SIGSTOP' in the sleep function. I could do that.

I hope now it's clear what I did and also what I do not know what to do (like the dbus). If you think there are parts that will benefit Back in Time, I would be very happy if you took them and put them to good use.
I think that I answered most if not all of your questions ,but if there is anything else please tell me if I can do anything.

@buhtz
Copy link
Member

buhtz commented Jun 9, 2024

Hello Fanis,

the indention sure is a problem, I do not take that lightly and I really apologise about that, this should not have happened.

I wouldn't phrase it like that. It can happen and also happens to me and others. But you should have fixed it immediately after I pointed you to it instead of discussing about it. It is still not fixed, isn't it?

I do not have any experience with the dbus
...
All I have done about this is to change the name from 'DBUS_SHUTDOW' to 'DBUS_SLEEP'.

So you copy and pasted code from the ShutDown class? And you don't know anything about DBUS?

My apologize but I don't see how we can bring this RP to an mergable end. The issue behind is to complex.

if there is anything else please tell me if I can do anything.

I would suggest you to pick another Issue from the BIT project. I am sure we can find one.

Best regards,
Christian Buhtz

@buhtz buhtz added Close after cooling-off period and removed Feedback needs user response, may be closed after timeout without a response PR: Modifications requested Maintenance team requested modifications and waiting for their implementation labels Jun 9, 2024
Copy link
Member

@buhtz buhtz left a comment

Choose a reason for hiding this comment

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

Again I need to point you to our communication problem.
I tried to explain that this PR won't get merged because of several reasons. But you are going on to work in it.
You wasting your time working on this PR so I close it now.

This is my last offer. Please pick an easier Issue, read what I write, improve the communication and I will guide you.

Best,
Christian

Comment on lines +24 to +27
name: Upload coverage reports to Codecov
uses: codecov/[email protected]
with:
token: ${{ secrets.CODECOV_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

This makes no sense and is far beyond the scope of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants