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

fix: add missing contexts to blueprint api methods [publish] #454

Merged
merged 1 commit into from
Feb 15, 2021

Conversation

Julusian
Copy link
Contributor

@Julusian Julusian commented Feb 9, 2021

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Fix

Ensures every blueprint method has a context parameter. Some got lost during conflicts in #287

  • Other information:

Status

  • Code documentation for the relevant parts in the code have been added/updated by the PR author
  • The functionality has been tested by the PR author
  • The functionality has been tested by NRK

@Julusian Julusian requested a review from gundelsby February 9, 2021 22:45
Copy link
Member

@gundelsby gundelsby left a comment

Choose a reason for hiding this comment

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

The code changes are fine, but the contexts that were removed were removed because they weren't used for anything (according to ESlint). So if they actually are necessary that means that our method signatures don't reflect that, which might be an issue we should look into

@Julusian
Copy link
Contributor Author

This is a 'public' api, so they should be there because they are useful, not only if they are used by our current blueprint implementation.
Additionally, the only way to log from the blueprints is via the context.logWarning and similar methods, so not having them impairs debugging when writing implementations of those methods

@Julusian Julusian changed the base branch from release30 to release31 February 11, 2021 19:41
@Julusian Julusian changed the title fix: add missing contexts to blueprint api methods fix: add missing contexts to blueprint api methods [publish] Feb 15, 2021
@Julusian Julusian merged commit 2cef36c into release31 Feb 15, 2021
@Julusian Julusian deleted the feat/missing-blueprints-contexts branch February 15, 2021 11:59
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