Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rename Java8 package from vm8 to v8 for consistency #562

Closed
wants to merge 1 commit into from

Conversation

jwagenleitner
Copy link
Contributor

No description provided.

@daniellansun
Copy link
Contributor

+1

@blackdrag
Copy link
Contributor

just two questions:
(a) why not from v8 to vm8 (vmX is the older one, so I was wondering)?
(b) why exclude v8 and vm8 in the build, if this is done to have only one of them?

@paulk-asert
Copy link
Contributor

I asked John to change the Java8 plugin package for consistency. We had used v5, v6, v7, and then vm8 (but so far only in 2.5.0-beta-1). I think 'vm8' is actually slightly more descriptive than 'v8' but doesn't match the convention now in many releases.

Since all tests containing vm8 in the core module were plugin related it seems okay to rename them too.

There are actually a few other spots in test names/packages using the string 'vm8' which we could also change to v8 but that can be done separately I think.

@jwagenleitner
Copy link
Contributor Author

After some more thought I wonder if this (2.5+) would be the time to start a org.apache.groovy.vmplugin or org.apache.groovy.internal.vmplugin package and start with a vm8 package under it establishing it as a new standard naming (i.e., followed soon by vm9)?

If there's interest in that I'll redo the PR, otherwise will merge this as is.

@paulk-asert
Copy link
Contributor

paulk-asert commented Jun 20, 2017

I think it would be confusing to have the plugins for different vm versions across different packages but I like your idea of doing a bit of a clean up. We are talking about a bunch of what should be internal classes, so I think we have some liberty to change but we'd probably want to do so with minimum disruption for anyone that might be sneaking in and using those "internal" classes.

I'd be inclined to leave the org.codehaus.groovy.vmplugin package as is but remove vm8 and deprecate everything else. Then create an org.apache.groovy.internal.vmplugin package as you suggest and bring across a merged Java7 class in the vm7 package which merges the current v5/v6/v7 classes (later versions taking priority). The vm8 package would of course reappear in the new package.

We'd need to double check that didn't alter anything but I believe should be what we want for 2.5+. The tests can all stay in the vmX package variants.

@blackdrag
Copy link
Contributor

In the end it does not matter all that much to me if we use v8 or vm8, just that it is only one of them ;)

Should we go with org.apache.groovy.internal.vmplugin? We can do that.

Frankly, some of the functionality in the plugins should probably move out of them again, since for example generics are really really part of all deployments of Groovy. But that is for a later PR.

@jwagenleitner jwagenleitner mentioned this pull request Jun 21, 2017
@jwagenleitner
Copy link
Contributor Author

PR #563 includes what I would think are "reasonably" compatible changes while moving to a new package and deprecating the old plugins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants