search

davidmhewitt / torrential

A simple torrent client for elementary OS
GNU General Public License v2.0
143 stars 30 forks source link

Show status info while a new torrent is being checked #127

Closed JessiAuro closed 5 years ago

JessiAuro commented 5 years ago

This is my first contribution to an open source project of any kind, so forgive me if I've made some obvious errors. I'm pretty new to Vala, but I've done a decent amount of OO programming, and I'm excited to break into software development.

This pull request is to fix #126. When a new torrent is added that has existing data on disk, the progress bar will turn yellow with the status text "Checking".

Implementing this required reworking the way CSS is applied to the progress bar in TorrentListRow, by adding the Gtk.CssProvider to the progress bar when it is initialized and simply updating the style as needed when update() is called.

This results in much less code duplication, though it may be inefficient if CssProvier's load_from_data() method is particularly expensive. I'm quite new to GTK.

I've tested it on my own system and everything seems to look good, though this request adds a new string that will need translation.

I've also attached a screenshot to give you an idea of what it looks like. Screenshot from 2019-03-16 12 31 47

davidmhewitt commented 5 years ago

For your first contribution to an open source project, this is great! Especially the detailed description of why you've done certain things. I see nothing wrong with the code at all.

However, I think I disagree that the checking state needs to be a different colour. I'm using the green colour to denote success, as per the CSS variable name. I don't think the checking state is a important enough state to need another colour as well as the "Checking" label.

So, if you get chance to revert the CSS changes so that the checking state is just a different label and not a different colour too, I'll happily accept this PR.

Thanks for your contribution!

JessiAuro commented 5 years ago

For your first contribution to an open source project, this is great! Especially the detailed description of why you've done certain things. I see nothing wrong with the code at all.

However, I think I disagree that the checking state needs to be a different colour. I'm using the green colour to denote success, as per the CSS variable name. I don't think the checking state is a important enough state to need another colour as well as the "Checking" label.

So, if you get chance to revert the CSS changes so that the checking state is just a different label and not a different colour too, I'll happily accept this PR.

Thanks for your contribution!

Alright, what would be the best way to revert the changes and modify the PR? Do I have to create a new PR?

davidmhewitt commented 5 years ago

You can either carry on working on this branch and put the relevant parts back to how they were, create a new commit and then push it.

Or you could create a new branch and make the non-css changes again. Whatever you feel is easiest :)

JessiAuro commented 5 years ago

Alright, I created a new branch called request, used git checkout -p master to only pull the parts i wanted, and then commited it to my repo. Do I have to modify this pull request or can you just incorporate the changes from my repo directly?

davidmhewitt commented 5 years ago

I've pulled in the changes, thanks again for your help!