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

Addressing the next round of feedback #8

Merged
merged 1 commit into from
May 9, 2019

Conversation

realModusOperandi
Copy link
Contributor

  • Revised the statement about why use Angular to be a more general "why use it instead of doing everything server side." Maybe that's closer to what you're looking for?
  • Added a statement that there are many files in an Angular application but the reader will only need to edit a few of them in order to accomplish the task. I also mention that the start directory can be used as a starting template for any Angular + Open Liberty application.
  • Updated the statement about the HttpClient to indicate it's optional and a convenience feature (SME review feedback #7).
  • Updated the ARTISTS_URL to not include the host/port (SME review feedback #7).
  • Updated some hotspots that I missed when I cleaned up whitespace.
  • Added an explicit import and implementation of the OnInit interface, as well as a short description of what it is (lifecycle hook) and what it does (fire when the component is intialized.) This isn't needed for the application to function, but it is good practice to do.
  • Removed redundant "start server" direction at the end of the guide.

I am unable to reproduce the 503 errors described in #7. When I try to visit the URLs in the message, my browser waits for awhile and times out. They look like they are being fetched by the Liberty Maven plugin, which I see is set to the latest release version in the pom. Not sure what effect if any it has on the build itself but I think the problem is serverside and the pom is set up correctly.

@realModusOperandi realModusOperandi self-assigned this May 9, 2019
@evelinec evelinec self-requested a review May 9, 2019 19:28
Copy link
Contributor

@evelinec evelinec left a comment

Choose a reason for hiding this comment

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

The changes look good. Will merge so we can review the changes on the staging site.

@evelinec evelinec merged commit 09b1c3e into OpenLiberty:dev May 9, 2019
@evelinec evelinec mentioned this pull request May 9, 2019
12 tasks
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