libreio / libre

Simple AWS gateway for storing and sharing files
http://libre.io
Other
7 stars 9 forks source link

#221 Initial design for directory functionality #300

Closed pinaf closed 9 years ago

pinaf commented 9 years ago

For #221

pinaf commented 9 years ago

@karato travis failure is unrelated to issue. see #299

karato commented 9 years ago

@pinaf Thanks for your pull request, let me find someone who can review it

karato commented 9 years ago

@krzyk review it

krzyk commented 9 years ago

@pinaf I have one comment regarding the todo

pinaf commented 9 years ago

@krzyk ok changed it. please take another look.

krzyk commented 9 years ago

@karato we are waiting for #299

karato commented 9 years ago

@karato we are waiting for #299

@krzyk yes, of course, we'll wait for #299

krzyk commented 9 years ago

@pinaf qulice fails with:

[ERROR] /src/main/java/com/nerodesk/takes/doc/TkDirCreate.java[50-50]: Perhaps 'base' could be replaced by a local variable. (SingularField)

[ERROR] /src/main/java/com/nerodesk/takes/doc/TkDirCreate.java[50-50]: Avoid unused private fields such as 'base'. (UnusedPrivateField)
karato commented 9 years ago

@pinaf we waited for #299, it is closed already

k0r commented 9 years ago

@yegor256 we might have to postpone this feature. directory implementation is going cause a few code challenges: directory deletion, moving files to another directory, and reorganizing files if the directory is deleted, etc. could be several tickets to scale this feature.

k0r commented 9 years ago

@yegor256 what do you think?

yegor256 commented 9 years ago

@karato indeed, let's postpone this

karato commented 9 years ago

@karato indeed, let's postpone this

@yegor256 OK, I labeled it as "postponed" tag

karato commented 9 years ago

@karato indeed, let's postpone this

@yegor256 right, I'll try to find someone else for this task

karato commented 9 years ago

@longtimeago review it

pinaf commented 9 years ago

@k0r @yegor256 not sure this was intentional or not, but ever since I have been added to the nerodesk team I have permission to merge the pull request...

k0r commented 9 years ago

@pinaf nerodesk team has write permission.

pinaf commented 9 years ago

@k0r so it was intentional?

k0r commented 9 years ago

@pinaf #271

yegor256 commented 9 years ago

@k0r please fix it. the team must have "read-only" permissions

k0r commented 9 years ago

@yegor256 fixed.

yegor256 commented 9 years ago

@k0r you can also remove me and @karato from nerodesk-read team

k0r commented 9 years ago

@karato please, be careful with the labels.

pinaf commented 9 years ago

@yegor256 shall we proceed with CR ?

longtimeago commented 9 years ago

@pinaf good question .. I'm also waiting for the answer

krzyk commented 9 years ago

@yegor256 and why was it reassigned? I think it should have been unassigned completely until @k0r decides we should go with directories

yegor256 commented 9 years ago

@longtimeago I think we should proceed, review and merge. It's a pull request, we shouldn't hold it open for long time. (no matter what the status of the project is)

k0r commented 9 years ago

@yegor256 what about this?

yegor256 commented 9 years ago

@k0r my bad, I thought it's an issue, not a pull request. It's not a good practice to keep pull requests in "on hold" status for longer. It will "expire" and we'll seriously damage the motivation of its author. It's always better to merge, no matter how "bad" is the code (well, it has to pass rultor and code review, of course).

k0r commented 9 years ago

@yegor256 postponing the feature isn't intended to demotivate the programmer, it involves other challenges we've previously discussed here.

yegor256 commented 9 years ago

@k0r yes, postponing a feature is perfectly fine, but it's a pull request, not a feature. there is already some code written.

k0r commented 9 years ago

@yegor256 thanks. proceed, if needed.

krzyk commented 9 years ago

@yegor256 in such case can we reassign it to back to me, as I already reviewed it and, the only thing that is left is saying "merge" :-)

longtimeago commented 9 years ago

@krzyk I don't mind :)

pinaf commented 9 years ago

@karato please assign @krzyk here

pinaf commented 9 years ago

@yegor256 could you assign @krzyk please?

yegor256 commented 9 years ago

@karato assign @krzyk to this pls

karato commented 9 years ago

@karato assign @krzyk to this pls

@yegor256 OK. @krzyk the ticket is yours, please proceed

krzyk commented 9 years ago

@rultor merge pls

rultor commented 9 years ago

@rultor merge pls

@krzyk Thanks for your request. @yegor256 Please confirm this.

yegor256 commented 9 years ago

@rultor merge

rultor commented 9 years ago

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor commented 9 years ago

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 10min)

karato commented 9 years ago

@krzyk Thanks a lot, I just topped your account for 21 mins, transaction ID 58657984, total time was 1 hour and 5 mins... extra minutes for review comments (c=6)... +21 added to your rating, at the moment it is: +4684

karato commented 9 years ago

@rultor deploy now pls

rultor commented 9 years ago

@rultor deploy now pls

@karato OK, I'll try to deploy now. You can check the progress here

rultor commented 9 years ago

@rultor deploy now pls

@karato Done! FYI, the full log is here (took me 3min)