Skip to content

Comments

Ready: Grant notes while onboarding to DSE and taking 102 courses.#45

Merged
MasonEgger merged 7 commits intomainfrom
grant-onboarding-findings
Mar 12, 2025
Merged

Ready: Grant notes while onboarding to DSE and taking 102 courses.#45
MasonEgger merged 7 commits intomainfrom
grant-onboarding-findings

Conversation

@GSmithApps
Copy link
Contributor

@GSmithApps GSmithApps commented Jan 2, 2025

What was changed

I just made some changes and fixed typos etc as I was taking the 102 courses.

How was this tested

I ran the code as I took the courses, and I left my fixes as I fixed or modified things.

Misc Note

I have a similar PR for the other languages in 102 code repo and content repo, so ~8 of these (4 * 2).

Comment on lines 42 to 44
TranslationActivityInput goodbyeInput = new TranslationActivityInput("goodbye", languageCode);
// TODO: Add a log statement here at the debug level stating that the Activity is going
// to be invoked. Be sure to include the word being translated and the language code.
TranslationActivityInput goodbyeInput = new TranslationActivityInput("goodbye", languageCode);
Copy link
Contributor Author

@GSmithApps GSmithApps Jan 8, 2025

Choose a reason for hiding this comment

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

definitely no biggie, but I just moved the instruction comment below where goodbyeInput was defined because if the learner puts it just below the note, it isn't defined yet

Copy link
Collaborator

Choose a reason for hiding this comment

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

We want the log to happen prior to the activity being invoked, that way we could look at the logs and know that if we haven't seen logs after that an activity is retrying. They should be logging the variables and not using the result from goodbyInput.

9. Save your changes
10. Add the following code at the bottom of the `TranslateActivityInput` class.
10. Add the following code at the bottom of the `TranslateActivityInput` class,
and add this import at the top of the file: `import java.util.Objects;`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The user could certainly figure this out, but I think they need to add this import for the code we're adding below to work

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not necessary. It's java.lang.Object. All tests in the solution directory run as expected.

Comment on lines -110 to +112
- The `helloMessage` field in the result is `Bonjour, Pierre`
- The `goodbyeMessage` field in the result is `Au revoir, Pierre`
- The `helloMessage` field in the output is `Bonjour, Pierre`
- The `goodbyeMessage` field in the output is `Au revoir, Pierre`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the variable is named output

that your microservice is running as stated above. Then run the test.

1. Run the `mvn test` command to execute the provided test
1. `cd` into `exercises/testing-code/practice/`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to cd in. not sure if we have already written that somewhere or if we assume the user can figure that out 👍

package translationworkflow;

import java.net.HttpURLConnection;
import java.net.ProtocolException;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this was being used

import java.io.IOException;
import io.temporal.activity.Activity;
import io.temporal.failure.ApplicationFailure;
import java.net.HttpURLConnection;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this was being used

Comment on lines -15 to -16
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Step B2 in the readme is to add these imports, so I think we want to remove them from the code 👍

@GSmithApps GSmithApps marked this pull request as ready for review January 8, 2025 19:59
@GSmithApps GSmithApps changed the title Grant notes while onboarding to DSE and taking 102 courses. Ready: Grant notes while onboarding to DSE and taking 102 courses. Jan 28, 2025
@GSmithApps GSmithApps requested a review from MasonEgger January 28, 2025 03:58
@MasonEgger MasonEgger merged commit 4182891 into main Mar 12, 2025
3 checks passed
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.

2 participants