Ubuntu File Manager App

Merge lp://staging/~whosdaz/ubuntu-filemanager-app/gci6198315054006272 into lp://staging/ubuntu-filemanager-app

  1. gci6198315054006272
  2. Merge into trunk
Proposed by Matthew Allen
Status: Rejected
Rejected by: Arto Jalkanen
Proposed branch: lp://staging/~whosdaz/ubuntu-filemanager-app/gci6198315054006272
Merge into: lp://staging/ubuntu-filemanager-app
Diff against target: 448 lines (+237/-175)
4 files modified
README.autopilot.md (+55/-0)
README.developers.md (+154/-0)
README.md (+14/-169)
README.translations.md (+14/-6)
To merge this branch: bzr merge lp://staging/~whosdaz/ubuntu-filemanager-app/gci6198315054006272
Reviewer Review Type Date Requested Status
Andrew Hayzen (community) code format Approve
Ubuntu File Manager Developers Pending
Review via email: mp+281324@code.staging.launchpad.net

This proposal supersedes a proposal from 2015-12-23.

To post a comment you must log in.
Revision history for this message
Andrew Hayzen (ahayzen) wrote : Posted in a previous version of this proposal

This looks good so far, however for the other READMEs we have opted to use the markdown format, so to remain consistent with the other coreapps could this be updated? Also the web links should be in the format [title](url), you can use the weather mp [0] for reference. Sorry this should have been mentioned in the code-in task, but it hadn't been updated.

0 - https://code.launchpad.net/~emailgirishrawat/ubuntu-weather-app/markdown-readmes/+merge/280787

review: Needs Fixing
Revision history for this message
Andrew Hayzen (ahayzen) wrote : Posted in a previous version of this proposal

3 minor inline comments, otherwise it looks good :-)

review: Needs Fixing
Revision history for this message
Andrew Hayzen (ahayzen) wrote :

LGTM :-)

I'll let the file manager developers check over this to ensure that the build/autopilot etc instructions are up to date before it is top approved.

Thanks!

review: Approve (code format)
Revision history for this message
Arto Jalkanen (ajalkane) wrote :

Hi,

thanks for this but there was recently another update merged to READMEs and I'm not sure if this merge proposal is valid anymore.

Could you check the current state of the trunk and see if there's any improvements that should be done regarding your changes here? I think this merge proposal is obsolete at the moment as there was quite a bit of restructuring.

Revision history for this message
Matthew Allen (whosdaz) wrote :

> Hi,
>
> thanks for this but there was recently another update merged to READMEs and
> I'm not sure if this merge proposal is valid anymore.
>
> Could you check the current state of the trunk and see if there's any
> improvements that should be done regarding your changes here? I think this
> merge proposal is obsolete at the moment as there was quite a bit of
> restructuring.

From what I can gather, I would agree with you. Wasn't like that when I originally cloned the repository and was still up on Google Code-In - sorry bout that!

Revision history for this message
Arto Jalkanen (ajalkane) wrote :

> > Hi,
> >
> > thanks for this but there was recently another update merged to READMEs and
> > I'm not sure if this merge proposal is valid anymore.
> >
> > Could you check the current state of the trunk and see if there's any
> > improvements that should be done regarding your changes here? I think this
> > merge proposal is obsolete at the moment as there was quite a bit of
> > restructuring.
>
> From what I can gather, I would agree with you. Wasn't like that when I
> originally cloned the repository and was still up on Google Code-In - sorry
> bout that!

Not your fault at all. These changes landed just before yours. Thank you and if you see anything in the current documentations that you could improve, please do submit another merge request!

I've rejected this now as it's not valid against current documents.

Unmerged revisions

515. By Matthew Allen

Fixed minor syntax issues

514. By Matthew Allen

Changed file extensions to .md

513. By Matthew Allen

Fixed Markdown Syntax

512. By Matthew Allen

removed old readme text

511. By Matthew Allen

fixed readmes

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches