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

refactor: include user and token service interfaces in index.ts #2875

Merged
merged 1 commit into from
May 13, 2019

Conversation

emonddr
Copy link
Contributor

@emonddr emonddr commented May 12, 2019

Include user and token service interfaces
in ./service.index.ts and ./index.ts

While refactoring the shopping cart application to use the latest authentication package, I noticed the newly added
user and token service interfaces in the services folder from the feat: design auth system with user scenario PR weren't added to an index.ts in the
services folder, and that the main index.ts doesn't include all exports from the ./services folder.

Before the shopping application refactor PR lands, this PR (which updates to the @loopback/authenticate package) must land.

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

@emonddr
Copy link
Contributor Author

emonddr commented May 13, 2019

this PR also removes an empty interface (session interface) we are not implementing at this time.

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

+1 to export these interfaces if they are needed by consumers.

IMO, this change is a regular feature (we are adding new public API), not a refactoring. The commit message should use feat type, not refactor.

See https://en.wikipedia.org/wiki/Code_refactoring (emphasis is mine):

Code refactoring is the process of restructuring existing computer code without changing its external behavior.

Copy link
Contributor

@jannyHou jannyHou left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@emonddr
Copy link
Contributor Author

emonddr commented May 13, 2019

@bajtos

IMO, this change is a regular feature (we are adding new public API), not a refactoring. The commit message should use feat type, not refactor.

Hi Miroslav, this PR is a quick fix to some new feature code (#2763) that was committed at the beginning of May. So can this PR still have refactor ? It doesn't change the external behaviour of the previous PR.

@emonddr emonddr force-pushed the dremond_export_auth_service_interfaces branch from cbdbf84 to 8d80270 Compare May 13, 2019 14:45
@emonddr
Copy link
Contributor Author

emonddr commented May 13, 2019

changed the commit type from refactor to fix based on a discussion with @bajtos .

Include user and token service interfaces
in ./service.index.ts and ./index.ts
@emonddr emonddr force-pushed the dremond_export_auth_service_interfaces branch from 8d80270 to c64ba06 Compare May 13, 2019 15:00
@emonddr emonddr merged commit 3a1a978 into master May 13, 2019
@emonddr emonddr deleted the dremond_export_auth_service_interfaces branch May 13, 2019 17:11
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.

5 participants