Skip to content

Conversation

@mtbc
Copy link
Member

@mtbc mtbc commented Feb 11, 2013

Small tweaks, partly caused by comments on PRs #669, #690.
Set Java source files to be UTF-8.
Also correct resource leakage in TempFileManager.

Small tweaks, partly caused by comments on PRs ome#669, ome#690.
@mtbc
Copy link
Member Author

mtbc commented Feb 11, 2013

Documentation update in ome/omero-documentation#245

@joshmoore
Copy link
Member

@mtbc: all of these were in your fs-path PR?

@mtbc
Copy link
Member Author

mtbc commented Feb 11, 2013

No, I'll have to rebase this to develop (but half of it should vanish in the process).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really fall under the description of this commit/PR. Probably fine, but do shoot for the principle of least surprise where possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I forgot I'd noticed this; adjusted description accordingly.

@jburel
Copy link
Member

jburel commented Feb 11, 2013

I will use one PR only to fix the UTF-8 stuf.

@mtbc
Copy link
Member Author

mtbc commented Feb 12, 2013

@jburel: You want me to break this PR into two, one with UTF-8 things, one with the other things?

@jburel
Copy link
Member

jburel commented Feb 12, 2013

@mtbc: since @manics opened a PR related to UTF-8, I thought it was better to do cover the code in the same PR

@manics
Copy link
Member

manics commented Feb 12, 2013

@jburel #690 fixed an incorrectly coded non-utf8 char on develop only which was causing compilation errors on some systems. This PR involves changing between two equally valid utf-8 representations.

@manics
Copy link
Member

manics commented Feb 12, 2013

Though we could wait for this to be rebased and just cherry pick 08c0721

@manics
Copy link
Member

manics commented Feb 12, 2013

What's the plan? Move the non-utf8 stuff into another PR?

@jburel
Copy link
Member

jburel commented Feb 12, 2013

@manics, @mtbc: I will vote for the UTF-8 changes in another PR.

@mtbc
Copy link
Member Author

mtbc commented Feb 13, 2013

Okay, I'll close this one and open a UTF-8 one and a non-UTF-8 one against dev_4_4.

What do you want me to do about #690 ? Should my rebased UTF-8 be a PR against the Unicode fix branch of @manics ? Or should he close that and I should cherry pick his commit across to a rebase against develop?

@mtbc mtbc closed this Feb 13, 2013
@mtbc mtbc deleted the cleanup branch February 13, 2013 08:33
@mtbc
Copy link
Member Author

mtbc commented Feb 13, 2013

Split into #729, #730, #731.

@manics
Copy link
Member

manics commented Feb 13, 2013

Cherry pick 08c0721 into your rebased develop PR. I'll close #690.

@mtbc
Copy link
Member Author

mtbc commented Feb 13, 2013

Will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants