lishid / OpenInv

Open anyone's inventory as a chest, real-time!
GNU General Public License v3.0
120 stars 97 forks source link

Suggestion: Versioning #59

Closed Narimm closed 7 years ago

Narimm commented 7 years ago

I really love the changes you guys are adding but can I make a suggestion. You havent versioned your work...ever...with the introduction of an api - I would have thought it a good idea. Once other plugin makers start using the api they are going to want to advertise a supported version and while it might be the case that the api will always work with version 1.0. As you add new end points etc that would I assume change....

Also do you host or publish the api to a maven repo? there is nothing in the POM. To automate maven its nice to host somewhere - I currently self publish to our maven repo we run but an official repo would be nice.

Narimm AddstarMC jenkins.addstar.com.au Artifactory

Jikoo commented 7 years ago

Good point. The reason I didn't use Maven's versioning system is that there are a ton of submodules, and while it's easy enough to write a script to edit every child pom, it always just seemed like a lot of fluff in the diff. I suppose we could always use Jitpack or some other similar service, the core plugin with no version-specific modules should build with a couple small pom tweaks.

Jikoo commented 7 years ago

Ok, from here on out Jitpack should be providing builds. I have yet to change versioning to match, largely because I'll need to add comments to differentiate the version declarations that will actually need to be changed and that's a decent bit of manual work. The API should be available using the following as a dependency:

    <dependency>
      <groupId>com.github.lishid.OpenInv</groupId>
      <artifactId>api</artifactId>
      <version>1.0-SNAPSHOT</version>
    </dependency>

Versioning for single modules does not (at least, according to JitPack's documentation) follow the same convention of allowing the usage of tagged releases or commit short hashes, so only 1.0-SNAPSHOT should be available for now. I have yet to test it, but I'm hoping to finish this up sooner rather than later.

Jikoo commented 7 years ago

Had some more time than expected today, so I've spent more time looking at this and thinking it over.

@Narimm how did you hope to see versioning implemented? There are a couple options: 1) Append API version to project version, 3.0.7-1.1.0 or something like that 2) API addition IOpenInv#getAPIVersion (moderately annoying and complex unless combined with something like option 1) 3) Do not version API separately, rely on project versioning being accurate (major version requiring API update, minor being new API features, and micro being the usual bugfixes/whatever)

Personally, I'd prefer the third option, but I've already failed proper versioning. OpenInv should have been 3.1.0 after d24827ffcb and probably 4.0.0 after removing IPlayerDataManager from the API in f05110c9b8. It would be very nice for dependent plugins to be able to say "I support this OpenInv major revision" and leave it at that.

I'm thinking we continue to use a property for versioning (or 2 if the API is versioned separately) and use it in all of the modules instead of leaving them at 1.0-SNAPSHOT constantly. It's considered bad practice to use a property to set versions, but it's a matter of updating a property versus updating a property and then running a script to update all poms. By my count we have 27 pom files, at least 26 of which would experience changes with every single release. That's a whole lot of fluff, and makes the real diff kind of a pain to read.

Narimm commented 7 years ago

Regarding API versioning - the API "can" be versioned seperately from the plugin assuming the plugin is backward compatible. In fact typically plugins actually depend on the API (because it contains all interfaces). So when the API changes (ie new features or deprecations) it needs versioning. A major version if you remove an api. You can minor version if you add features. Once the minor version is stable remove the qualifier (ie SNAPSHOT) or perform a API release.

Your plugin would then be built against the API release. You dont really need to worry too much about recording outside of the above which plugin version works with which API then because maven automatically finds the child dep(and the conflict if it exists for the end users)

Now regarding versioning all together - the way I do it - is use Maven release plugins...we basically hold everything as a 0.0.0-SNAPSHOT(ie MAJOR.MINOR.IncrementalVersion-QUAL) and when we are ready to release we run a maven release. which automatically will check the snapshot builds make a git commit with the new version ON ALL the modules (children included) ..incidently dont declare a version on children just let them inherit the parent module. That way only 1 pom changes(the parent for a release). Our plugin versions are filtered based on the ${project.version}-${build.number} from our ci server.

Regarding how to start..given the changes.. 2.0.0-SNAPSHOT. by my count you have 1 parent POM https://github.com/lishid/OpenInv/blob/master/pom.xml and under this 4 submodules (none of those should declare a version) neither should ANY submodules underneath them.

Now to update dependencies where a submodule depends on another module you can use the <version>${project.parent.version}</version> to ensure it always uses the right version.

I actually performed a release of a library we maintain tonight here the two commits it made (automatically) to github are https://github.com/AddstarMC/dhutils/commits/master (shows all the commits) https://github.com/AddstarMC/dhutils/commit/2b0347368edae4418e2f5f4ffdf59bf2faac1398 https://github.com/AddstarMC/dhutils/commit/c0556f75a93af6eccec03bd76d7a22e79f07d03e

Its a long read but it shows what executing a maven release is like: the commandline I used was <===[JENKINS REMOTING CAPACITY]===>channel started mvn: -B -f /ci/jenkins/home/workspace/DHUtils/pom.xml -DdevelopmentVersion=2.19.4-SNAPSHOT -DreleaseVersion=2.19.3 -DignoreSnapshots=true -Dresume=false release:prepare release:perform

Narimm commented 7 years ago

I should add I would probably just keep the API at say 2 and then all plugin versions are 2.0.0 compat...ie plugin version 2.1 would work with api 2.0.0.

You still version the api module inline with your normal versioning..but its unstated that 2.0 is forward compat with all plugin versions until you hit 3.0

Or you say 2.0 is compatible until 2.1 ie it would be fine with 2.0.1. You just state it in the readme.md, External devs will read it to get updates.

Narimm commented 7 years ago

If you like I can submit a patch to get all the versioning sorted...do you have a CI server if so what is it. Let me know what version you want to start at going forward

Jikoo commented 7 years ago

I should add I would probably just keep the API at say 2 and then all plugin versions are 2.0.0 compat...ie plugin version 2.1 would work with api 2.0.0.

You still version the api module inline with your normal versioning..but its unstated that 2.0 is forward compat with all plugin versions until you hit 3.0

Or you say 2.0 is compatible until 2.1 ie it would be fine with 2.0.1. You just state it in the readme.md, External devs will read it to get updates.

Yeah, that was my intent with option 3, which makes the most sense to me, I just wanted to make it clear that I've already made a couple mistakes in versioning, not bumping minor when I should have. I am familiar with how versioning is supposed to be done, and I'm quite familiar with my failings on that front.

Regarding how to start..given the changes.. 2.0.0-SNAPSHOT. by my count you have 1 parent POM https://github.com/lishid/OpenInv/blob/master/pom.xml and under this 4 submodules (none of those should declare a version) neither should ANY submodules underneath them.

Given that the plugin itself is already at 3.0.6, one assumes a version higher than that would be better for clarity for end users :p We'll probably start off at 3.1.0-SNAPSHOT

If you like I can submit a patch to get all the versioning sorted...do you have a CI server if so what is it. Let me know what version you want to start at going forward

I don't have a CI server, no, that's why I resorted to JitPack for API builds. I'd typed that up in my original reply, but that was lost.

Edit: Oh, looks like Maven flat out won't let me do what I wanted any more. We'll resort to the usual Maven version bumping. It's gonna be great.

Jikoo commented 7 years ago

Unfortunately, using ${project.version} for the version of a dependency appears to break our assembly process. I'll just have to suck it up and deal with the large useless diffs.

Narimm commented 7 years ago

it should be ${project.parent.version}

Jikoo commented 7 years ago

it should be ${project.parent.version}

Tried both, neither worked for me using Maven 3.3.9. Should have clarified. The assembly plugin is incredibly finicky.

Narimm commented 7 years ago

Wierd I run the same version under IntelliJ and it builds fine using those params.