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

Move extensions to Kotlintest-Extensions module #629

Merged
merged 2 commits into from
Jan 31, 2019

Conversation

LeoColman
Copy link
Member

This commit moves all extensions to Kotlintest-Extensions module. This is necessary because as we add more extensions to the code, the Core module would get fuller by this. By keeping all extensions in the same place, it's easier to search and to refactor.

The moved extensions are safe to move, because neither SystemProperties nor LocaleExtensions, nor TimezoneExtensions are currently in production, therefore no users are using them yet.

All related tests were moved accordingly.

An important point to note is that the JVM-Runners module is now depending on the extensions modules. We did this to avoid users having to use more than one import in their dependencies. External/Integration extensions are still kept in their own modules, such as Spring and Allure.

Closes #625

@LeoColman
Copy link
Member Author

@sksamuel
My only concern is that I might have screwed up the maven modules and jars. I'm not sure how they work completely

@LeoColman LeoColman force-pushed the feature/625-rework-extensions-module branch 2 times, most recently from 810b077 to 28dd00b Compare January 31, 2019 00:45
@@ -1,5 +1,4 @@
dependencies {
compile project(':kotlintest-assertions')
Copy link
Member

Choose a reason for hiding this comment

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

Do we not want this now ?

Copy link
Member Author

Choose a reason for hiding this comment

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

IDK if we want it, but we don't need it anymore.
We can add it, no difference to me, but it's not necessary

Copy link
Member Author

Choose a reason for hiding this comment

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

I can add it back, no problem.
I just thought that maybe it's not a dependency to core anymore

Copy link
Member

Choose a reason for hiding this comment

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

Why was it, I can't remember.

Copy link
Member Author

Choose a reason for hiding this comment

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

Due to SysProps using something from core, if I remember correctly

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't look like it. I may have done it just because I'm happy to have core depend on it. No reason :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think it's better to have it? I'm happy to add it too!

This commit moves all extensions to Kotlintest-Extensions module. This is necessary because as we add more extensions to the code, the Core module would get fuller by this. By keeping all extensions in the same place, it's easier to search and to refactor.

The moved extensions are safe to move, because neither `SystemProperties` nor `LocaleExtensions`, nor `TimezoneExtensions` are currently in production, therefore no users are using them yet.

All related tests were moved accordingly.

An important point to note is that the JVM-Runners module is now depending on the extensions modules. We did this to avoid users having to use more than one import in their dependencies. External/Integration extensions are still kept in their own modules, such as Spring and Allure.

Closes #625
@LeoColman LeoColman force-pushed the feature/625-rework-extensions-module branch from 28dd00b to a914fd4 Compare January 31, 2019 03:02
@LeoColman LeoColman merged commit 2a7295f into master Jan 31, 2019
@LeoColman LeoColman deleted the feature/625-rework-extensions-module branch January 31, 2019 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants