Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add support for Spring boot 2.1.0 and Lovelace SR2 #213

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

Merged
derjust merged 12 commits into derjust:Issue_218 from boostchicken:master
Nov 28, 2018

Conversation

@boostchicken
Copy link

@boostchicken boostchicken commented Oct 23, 2018

No description provided.

Copy link
Author

I cant run format on windows its add windows line endings

Copy link

codecov-io commented Oct 26, 2018
edited
Loading

Codecov Report

Merging #213 into master will increase coverage by 0.19%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@
## master #213 +/- ##
===========================================
+ Coverage 65.21% 65.4% +0.19% 
- Complexity 598 600 +2 
===========================================
 Files 77 77 
 Lines 2038 2038 
 Branches 374 374 
===========================================
+ Hits 1329 1333 +4 
+ Misses 554 550 -4 
 Partials 155 155
Impacted Files Coverage Δ Complexity Δ
.../repository/support/DynamoDBRepositoryFactory.java 85.1% <ø> (ø) 20 <0> (ø) ⬇️
.../data/dynamodb/mapping/DynamoDBMappingContext.java 75% <0%> (+25%) 8% <0%> (+2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f4ba4a...77d9206. Read the comment docs.

Copy link
Author

Loss of code coverage is due to Spring making more interfaces final making Mockito useless.

Copy link
Author

Loss of code coverage issues resolved

Copy link
Author

@derjust any intention of throwing up this snapshot Spring 2.1 is GA and I would love to move away from a local repo

Copy link
Owner

derjust commented Nov 6, 2018
edited
Loading

Appreciate it - looks good!

Right now thinking about the various versions that are out there to be supported - honestly dislike that they introduced a new method signature again.

Merging with #17

boostchicken reacted with thumbs up emoji

@derjust derjust mentioned this pull request Nov 9, 2018
Copy link
Author

Agree plus all the new final classes, was not hard to get it working, but I shouldn't have had to make any changes at all

Copy link

ryonday commented Nov 13, 2018

Looks like we independently did much the same work in our respective PRs!

I did manage to get the Mockito stuff working.. Looks like between your PR and mine we can keep the tests and gain Spring Boot 2.1.x compatibility!

Perhaps to make things easier on @derjust we can combine our two PRs

Copy link
Author

boostchicken commented Nov 15, 2018
edited
Loading

I am fine with whatever, I just needed it for my project so I did it, looks like we came to same solution although I think I handled the code coverage a bit different, do a diff and you can see. I think mine is more the "Mockito" way, I could be wrong though. Been a while since I used Mockito.

@ryonday @derjust

Copy link
Author

Also I have some fixed to get it to build on windows environments properly in the POM as well, I know that's rare but it's still useful :)

Copy link
Author

@ryonday let me know if you want to merge into my branch and PR or go the other way around and we'll get the permission sorted and what not.

Copy link
Author

Also @derjust if you need some help administering the project let me know, I've contributed to a few spring data projects in the past (solr, jpa, now dynamo)

Copy link

@devinsba devinsba left a comment

Choose a reason for hiding this comment

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

I hope you don't mind @derjust but my team needs this code too so I figured another review might help you finalize this

Copy link
Author

Incorporated @devinsba feedback

@boostchicken boostchicken changed the title (削除) Add support for Spring boot 2.1.0 and Lovelace SR1 (削除ここまで) (追記) Add support for Spring boot 2.1.0 and Lovelace SR2 (追記ここまで) Nov 21, 2018
Copy link
Author

@derjust just tested this all against SR2 to be sure and we are good.

@derjust derjust changed the base branch from master to Issue_218 November 28, 2018 04:37
@derjust derjust merged commit 77d9206 into derjust:Issue_218 Nov 28, 2018
Copy link
Owner

derjust commented Nov 28, 2018

A thousand thanks to all!
@boostchicken @ryonday @devinsba all appreciate your work!
I merged all of those into branch derjust:Issue_218 to keep all your commits intact & provide credits.

Any help is always highly appreciated and I hope to have more free time soon again.

Still interessted in the changes for a Windows build :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@devinsba devinsba devinsba left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /