You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/03/28 17:05:01 UTC

[GitHub] [maven] mkarg commented on pull request #421: Artifact.getPath() and .setPath()

mkarg commented on pull request #421:
URL: https://github.com/apache/maven/pull/421#issuecomment-808925420


   The contract is:
   * There is *one single* property, which is named `file`, and it has the getters and setters `getFile` and `setFile`, and the contract is that `getFile` returns `setFile`. This already existed and is tested, so there is no need to test *that* contract again.
   * There is **no** separate property named `path`. `getPath` simply is *an alias* for `getFile`, and `setPath` is *an alias* for `setFile`, working on the existing (and tested) property `File`, but certainly doing conversion on the fly. This is **by intention** because the idea is **not** to have *two* properties, but to provide a smooth transition phase from the IO-era to the NIO-era, i. e. to **replace** `File` some fine day in the future.
   
   Hence it makes no sense to tests *again* whether the property works as expected, but it makes much sense to check if the alias really works as an alias.
   
   In addition, my new test *solely* tests the code *that I added*, and it only tests it for *what I itend to*. Hence, it proofs whether the new alias methods are there, and whether they really act as an *alias*. I deliberate *do not* test if `getPath` returns `setPath` as this would imply testing the `file` property (which is already tests).
   
   Also it is irrelevent if *some other* implementation of the interface would fail the tests, as I deliberate *do not* tests *some* implementation, but in fact, I *only* test the default alias implementation (not *any* alias implementation). Testing *other* implementations, or testing *other* scenario simply is out of scope of this PR, which *solely* has the intention to add the alias methods to the interface, if you remember.
   
   So if there is anything unclear about the intention of this PR, or if somebody has really a huge problem with this, then please tell now and clearly **what** problem does **actually** exist. Does this PR provide harm? No. Is it beneficial? Yes.
   
   Don't let us squander valuable time arguing about things *beyond* the scope of this PR. Let's focus on the merits of **this PR**, and let's move on, even if we have different likes or dislikes.
   
   Otherwise I would really like to kindly ask you to merge this PR (I am not a committer, someone of you need to do it). Thanks.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org