GNOME Bugzilla – Bug 698933
places sidebar: don't capitalize "file system"
Last modified: 2013-05-13 09:46:31 UTC
The places sidebar currently provides this tooltip for the Computer item I see in Nautilus: Open the contents of the File System There's no reason to capitalize "file system" here. It's not a proper noun, and tooltips generally use sentence capitalization.
Created attachment 243175 [details] [review] Changed all ocurrences of "File System" to "file system".
I agree with this one. I skimmed through the .po files and from all the languages I saw, only a small percentage of the languages used Upper Case while the vast majority all used lower case for their version of "file system". As practice for my GSoC 2013 application, I've made and attached my first ever patch file. :>
Created attachment 243181 [details] [review] Changed "File System" to "file system". I just realized that the .po files are generated automatically, and that the functionality for this has recently been moved to gtk+. I've attached a patch for gtk+ to change "File System" to "file system".
What I meant to say in my previous comment was that this functionality has been moved from nautilus to gtk+ (my first patch was mistaken (horribly (I'm learning :]))).
Created attachment 243184 [details] [review] The previous patch changed unnecessary blank lines. This one just changes only the line in question. My vim set up was removing trailing white space on save when I made the last patch, so there was a few white spaces replaces with nothing in the previous patch. This one's fixed.
Review of attachment 243184 [details] [review]: trusktr, note that usual practice in GNOME is not to include the bug number in the summary line, but rather to include the full URL of the bug later in the commit message. You might want to glance at the git log for this repository for examples. Also, we don't generally use a trailing period in the summary line. See https://live.gnome.org/Git/CommitMessages . Otherwise this looks fine to me, but you shouldn't push until you get an OK from one of the actual GTK developers (I am not one).
Created attachment 243187 [details] [review] Fixed my commit message, otherwise same as before. Thanks Adam. I've fixed it. How does this one look?
Review of attachment 243187 [details] [review]: trusktr, Your summary line is too long - it should be at most 50 characters. See https://live.gnome.org/Git/CommitMessages . For such a trivial change you don't need to include much detail. I think a summary such as 'Fixed capitalization in tooltip' would be fine. If anyone wants to know more, they can just glance at the diff.
Created attachment 243355 [details] [review] resubmission. Thanks for the feedback Adam. How's this one?
Created attachment 243356 [details] [review] Fixed capitalization in tooltip Same file as before, but made a better attachment description. How does it look?
Looks good to me. Again, one of the GTK developers should give the OK to push this.
Cool. I don't have permission to push yet anyway. Now that I'm getting the hang of how patching and committing works, I'll try solving a bug that involves actual code changes. Should I just leave this bug as is until a GTK developer reads it?
Yes. One of the GTK devs will probably push this when they get a moment to do so.
One more point: you should have an actual first and last name listed as the author of this patch. If you want to remain anonymous I suppose that might be possible - I don't know what GNOME's policy is on accepting anonymous contributions, actually. But as you can see in our git logs normal GNOME practice is to record authors' full names.
Created attachment 243877 [details] [review] Fixed capitalization in tooltip K, I fixed my git user.name so it shows my name (Joe Pea), but when I created the new commit, somehow Matthias Clasen's name shows up instead. Also, some extra stuff has gotten in to the commit for configure.ac, gdk/Makefile.am, and gtk/Makefile.am (somehow) though I've done git reset --hard and the commit before I commited is 92597da3a02cd4c7b3847a517931453209ba075d (Add visibility flags in gtk/a11y) by Matthias Clasen.
Joe, You must have made some sort of error when you merged in recent changes from git master with your own changes. But don't feel bad - it's really easy to do that especially if you're still learning git. Because this is a tiny change, I think the easiest path forward here will be to restore your git tree to a completely pristine state: $ git reset --hard origin/master Be warned that that will delete all changes you've made in your working copy *and* all changes you've committed locally. Once you've done that, you can simply make this change again, then use git format-patch to create a new patch.
The code change in comment #10 looks fine to me. As Adam says, it would be great to have a patch that has your name on it, and has a meaningful summary and description. If I were to do it, I'd probably say something along the lines of: GtkPlacesSidebar: Fix tooltip capitalization Use sentence capitalization for the tooltip on the file system. https://bugzilla.gnome.org/show_bug.cgi?id=698933
Created attachment 243951 [details] [review] GtkPlacesSidebar: Fixed tooltip capitalization Here's the patch, updated with my name. @Adam, That's strange, I've done the exact same steps as before, but it simply worked this time. @Matthias, Thanks, I took your advice on the description.
Attachment 243951 [details] pushed as 67c8ebc - GtkPlacesSidebar: Fixed tooltip capitalization