Skip to content

Conversation

UgmaDevelopment
Copy link

Having never done Spring RestDocs, I thought I'd give it a try and fix anything I came across.

I started to fixed a few unclear portions, but then I ran into many broken things in the tutorial and had to abandon ship. Though I thought it would helpful to pass along the few fixes I did make.

It appears that this tutorial needs to be reworked. @Buzzardo it seems you may have accidentally broken some things around commit 42f92f2. I agree with you that this tutorial should be updated to use a more recent template from Spring Initializr. There's a violation of expectations, however, when you start off with just the Web starter and then end up with many more dependencies and configuration that appears out of nowhere, without any instruction. I think the simplest way to go about this would be to start off having all the dependencies included by Spring Initializr—perhaps RestDocs didn't use to be in the Initializr, but it is now—after all, that's what it's for, right?

I think starting off with Web and RestDocs as dependencies would simplify the changes that need to be made to make this tutorial workable again.

Thanks for all the hard work that's gone into this so far. I'd be happy to test out any future changes for continuity, with my naïve eyes.

Previously, it said that Spring created a `main` class.
Now it says that Spring creates a `main` function in an Application class.
…tion to run the main method was lost; now it's back.
It appears that the tags were accidentally broken by Jay Bryant <jbryant@pivotal.io> in commit 42f92f2
@@ -28,6 +28,7 @@
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
<!-- We'll be using JUnit 5. Vintage is for JUnit 4 so we exclude it. -->
<exclusions>
Copy link
Author

Choose a reason for hiding this comment

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

Excluding vintage seems like a good idea, but it doesn't come along with Spring Initializr. We just need to add a part in the tutorial to add this exclusion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Better simply to omit this part - it shouldn't be necessary with latest version of Spring Boot.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the build.gradle needs to be fixed though.

Copy link
Author

Choose a reason for hiding this comment

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

@dsyer, I've got a pull request aimed at starting this tutorial using the Spring Initializr (#11). That should fix any build concerns.

What do you think about that course of action?

Copy link
Collaborator

Choose a reason for hiding this comment

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

So this PR is redundant?

Copy link
Author

Choose a reason for hiding this comment

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

@dsyer, fixing the build in both places would be redundant, but no the PR is not redundant. This PR was mostly about fixing wording issues and adding clarifications; the other PR is focused on starting using Initializr.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, but the exclusion is redundant still? I mean there’s something that needs fixing, I guess I don’t care which PR it happens in.

Copy link
Author

Choose a reason for hiding this comment

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

It looks like after Spring Boot 2.4, the exclusion is redundant.

But, in this PR the version is still 2.3.2.RELEASE, so excluding vintage here is not redundant.

In #11, we move this tutorial to the latest version from Initialzr which is 2.4.1; but since we'll then be using the POM and other build files provided by Initializr, there's no problem there.

Copy link
Author

Choose a reason for hiding this comment

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

So I was mistaken earlier when I said

We just need to add a part in the tutorial to add this exclusion.

Adding the exclusion isn't necessary with 2.4.1 and if you happen to be using an older version from Initialzr, it will exclude it if necessary.

@dsyer
Copy link
Collaborator

dsyer commented Dec 12, 2020

Thanks for the updates. I made a few comments. I'm sorry you gave up because it all works for me.

Copy link
Contributor

@Buzzardo Buzzardo left a comment

Choose a reason for hiding this comment

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

Thanks for catching my goofs.

@UgmaDevelopment
Copy link
Author

UgmaDevelopment commented Dec 14, 2020

Thanks for the updates. I made a few comments. I'm sorry you gave up because it all works for me.

🤔 I'm curious if you followed along from the beginning of the tutorial or if you jumped to the end. I'd be willing to bet that the finished code is indeed functional. However, it appears that most Spring tutorials allow readers two ways of completing them:

  1. by following along from the beginning
  2. jumping to the part where it already works

My concern in this tutorial is with the first: the beginning doesn't appear to lead you very well to the end.

@UgmaDevelopment
Copy link
Author

UgmaDevelopment commented Dec 14, 2020

I've started a separate pull request (#11) regarding an effort to make it so that one could follow along from the beginning of the tutorial. It's not done, but feel free to make suggestions or additions.

Thanks!

@Buzzardo
Copy link
Contributor

Yes, we do try to offer two paths: One that walks the reader through each step and one that lets them jump to a completed project. We want both to be right, of course. We also want the tutorial to be as easy to follow as possible. We really appreciate when the community takes the time to test the tutorial and give us feedback, as you've done here. As with anything else, we can get only so far by ourselves. A good review gets us much further along. Thanks much.

@gregturn gregturn changed the base branch from master to main April 27, 2021 14:35
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.

3 participants