Re: [PR] [IO-808] non writeable destinations are I/O errors [commons-io]
garydgregory merged PR #582: URL: https://github.com/apache/commons-io/pull/582 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [IO-808] non writeable destinations are I/O errors [commons-io]
elharo commented on PR #582: URL: https://github.com/apache/commons-io/pull/582#issuecomment-1935084739 > > Should I go ahead and remove the requireCanWrite method? > > I think so, yes. done -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [IO-808] non writeable destinations are I/O errors [commons-io]
garydgregory commented on PR #582: URL: https://github.com/apache/commons-io/pull/582#issuecomment-1934797579 > Should I go ahead and remove the requireCanWrite method? I think so. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [IO-808] non writeable destinations are I/O errors [commons-io]
elharo commented on PR #582: URL: https://github.com/apache/commons-io/pull/582#issuecomment-1934672359 This PR seems unlikely to require source changes by users. These methods can already throw IOExceptions and those need to be handled. The only potential problem, and it's not a likely one, is if someone has a separate IllegalArgumentException catch block with special logic to handle read only files. In that case they'd need to move that logic into a block that catches IOException. However, I think more users are likely to be helped by combining IOException into a single exception type. As to simply removing the writeable check completely, that's a definite maybe. It would simplify the code, and simplify the happy path where there's no problem with read-only files. Public API remains the same. It's a very good point that this risks being out of sync with the fiel system by the time we write. Should I go ahead and remove the requireCanWrite method? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [IO-808] non writeable destinations are I/O errors [commons-io]
sebbASF commented on PR #582: URL: https://github.com/apache/commons-io/pull/582#issuecomment-1934554308 This PR just changes the Exception that is thrown. This may require source changes by users; is it really worth the effort? Although it may appear that IOE is more logical, there is an argument to be made that the cause is the choice of argument; i.e. the argument is illegal for the operation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [IO-808] non writeable destinations are I/O errors [commons-io]
garydgregory commented on PR #582: URL: https://github.com/apache/commons-io/pull/582#issuecomment-1934521716 A question is whether we should even check for writable attributes before we write. There comments from time to time that our APIs are (obviously IMO) non-atomic, and these checks make the code fail faster but also risk of being out of sync with the underlying file system. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [IO-808] non writeable destinations are I/O errors [commons-io]
elharo opened a new pull request, #582: URL: https://github.com/apache/commons-io/pull/582 No API changes. No existing tests failed. I added a new test for this. @garydgregory -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
