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

Proofread the tutorial #156

Merged
merged 4 commits into from
May 23, 2024
Merged

Conversation

gsmet
Copy link
Contributor

@gsmet gsmet commented May 22, 2024

I went through all the content and made the following changes:

  • fixed typos
  • reported some issues that were not that easy to fix (for instance the use of dead punkapi.com in some sections)
  • adjusted the first tutorials to use Jackson instead of JSON-B: we recommend using Jackson. Also it avoids having to go back to switch from JSON-B to Jackson when dealing with Spring
  • the reactive sections are still using JSON-B for two reasons: they are using some JSON-P features and I couldn't test things given the API is not available. When reworking them, we should switch them to Jackson too.

This is a bit massive, I will go through the changes and add comments when they are needed.

I went through all the content and made the following changes:
- fixed typos
- reported some issues that were not that easy to fix (for instance the
  use of dead punkapi.com in some sections)
- adjusted the first tutorials to use Jackson instead of JSON-B: we
  recommend using Jackson. Also it avoids having to go back to switch
  from JSON-B to Jackson when dealing with Spring
- the reactive sections are still using JSON-B for two reasons: they are
  using some JSON-P features and I couldn't test things given the API is
  not available. When reworking them, we should switch them to Jackson
  too.

This is a bit massive, I will go through the changes and add comments
when they are needed.
Copy link
Contributor Author

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

@kdubois Probably a good idea to add whitespace when reviewing. Small cog above the diff / Hide whitespace / Apply.

/cc @geoand @cescoffier

INSERT INTO Fruit(id,name,season) VALUES (7,'Watermelon','Summer');
INSERT INTO Fruit(id,name,season) VALUES (8,'Apple','Fall');
INSERT INTO Fruit(id,name,season) VALUES (9,'Pear','Fall');
ALTER SEQUENCE fruit_seq RESTART WITH 10;
----

And append the following configuration in `application.properties`:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This shouldn't be needed but... there is a bug and it's still needed. I'll figure out what's going on and will adjust in a followup.

documentation/modules/ROOT/pages/07_spring.adoc Outdated Show resolved Hide resolved
@gsmet
Copy link
Contributor Author

gsmet commented May 22, 2024

@kdubois once you agree on the changes, let me do a pass to check that I didn't break the formatting (unclosed backticks for instance).
I will do it locally but let's agree on the changes first so that I don't do it twice.

@kdubois
Copy link
Contributor

kdubois commented May 22, 2024

Looks great, thanks for going through and fixing / improving the content! I like the switch to Jackson as well.

@gsmet
Copy link
Contributor Author

gsmet commented May 23, 2024

@kdubois OK, I went through the content again, and went through the all the steps I changed (so until REST Client).

Things work but I had some bad surprises with dev mode when adding the extensions. Once, dev mode somehow didn't reload the model properly and I ended up with some classes (the MicroProfile REST Client ones) being missing.

Even using s didn't fix it.

It worked after a full restart of the dev mode.

I'll try to reproduce this but it doesn't seem fully reproducible and is not related to my changes so I would say all these changes are good to go.

@gsmet
Copy link
Contributor Author

gsmet commented May 23, 2024

@kdubois I added a small step to make sure people wait for Quarkus to be reloaded after having added an extension. Once PostgreSQL is installed, you have the Dev Service starting and it's relatively slow.

I think that might explain what I saw (but we should try to see how we could fix it to survive the issue)

@gsmet
Copy link
Contributor Author

gsmet commented May 23, 2024

@kdubois it would be nice to get this merged and published before next week as we are going to use this for the workshop at JCon.

Thanks!

@kdubois
Copy link
Contributor

kdubois commented May 23, 2024

LGTM!

@kdubois kdubois merged commit d228ba1 into redhat-developer-demos:master May 23, 2024
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