[silme] 'path' in FileClient.write_object should support both directories and files

RESOLVED FIXED

Status

Mozilla Localizations
Infrastructure
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: stas, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
Created attachment 369841 [details] [diff] [review]
Patch

I think that all across Silme, when working with l10nobjects, 'path' is the full path to the file, except for the FileClient.write_object class method. This method requires that 'path' point to a directory, and than appends the filename via object.id. So if you have a variable with the path to a file, you have to os.path.dirname it. Not a major pain, but would be nice to unify this.

Proposed patch is attached.
Attachment #369841 - Flags: review?(gandalf)
I'm not convinced, but I'll investigate it more tomorrow.

It definitely will have to be patched against the tree after bug 479155 gets solved.
Depends on: 479155
I agree with Stas here: it's better to have it here the same way like everywhere else.
Stas: 
1) The patch needs to be updated... ;) 
(the code for choosing the name is here:
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/file.py#l98
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/file.py#l84
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/file.py#l71
)

2) Something I was thinking about:
os.path.isdir(path) will be always True when writing a whole L10nPackage, because we create this path-directories here: http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/file.py#l117
So in case of writing a L10nPackage, we will use always a object.id as file name.
The question is: is there an other (working) way a developer can submit names for objects to be written...?
I think not, but I'm asking just to not be surprised when it's too late ;)
the question is if structure.id should be related to its file name.

I believe it should, but maybe there are some compelling reasons to split it in the future.
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/rev/81ad67f20b0c
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Reporter)

Updated

6 years ago
Attachment #369841 - Flags: review?(gandalf)
You need to log in before you can comment on or make changes to this bug.