-
Notifications
You must be signed in to change notification settings - Fork 13
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
Feature: New Backend System Migration #192
base: main
Are you sure you want to change the base?
Feature: New Backend System Migration #192
Conversation
Signed-off-by: Javier Prieto <[email protected]>
…s to comply with new backend Signed-off-by: Javier Prieto <[email protected]>
…o comply with new backend
🦋 Changeset detectedLatest commit: ae936fa The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
2cc98f0
to
6c2c41c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much, I have some minor concerns!
|
||
/** | ||
* Constructs a jira dashboard router. | ||
* @deprecated Please migrate to the new backend system as this will be removed in the future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "@Deprecation" notice here is a warning that the "RouterOptions" and "createRouter" should not be included in the public api of the plugin, as described here.
So we either need to remove the exports or keep the "deprecations"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say let's go ahead and remove the exports. You already had the deprecated tags in the latest version. According to the documentation, the next step is to remove the exports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I would be surprised if anyone was using the "old backend" still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd even go as far as to stop functions that were previously used to create a legacy-wrapper to be exported by the plugin, as suggested by the docs . I'm commiting these changes too, let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed too that as a best practice, all types are usually exported from the common plugin. How about I move those to the types file in the common plugin and export all types from there? That way, the backend plugin will be def cleaner and up to code!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not move any types right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anicke , all types have been moved back into the jira-dashboard-backend plugin.
…notification Signed-off-by: Javier Prieto <[email protected]>
export type { SearchOptions } from './api'; | ||
export { jqlQueryBuilder } from './queries'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can only remove createRouter and RouterOptions, i.e. what is deprecated. The others should be there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I'll put them back.
6d970d5
to
3f8ae36
Compare
@@ -93,8 +93,6 @@ export type SearchOptions = { | |||
* @param config - A Backstage config | |||
* @param jqlQuery - A string containing the jql query. | |||
* @param options - Query options that will be passed on to the POST request. | |||
* | |||
* @public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add the "@public" here? The "searchJira" is not included in the "api-report"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right... I just aded it back.
Signed-off-by: Javier Prieto <[email protected]>
Hey, I just made a Pull Request!
This feature provides full integration with the new backend system.
Context
Given that Backstage is rapidly evolving, this plugin was in danger of falling behind and starting to have notable incompatibilities.
Issue ticket number and link
Checklist before requesting a review