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

Feat/migration scaffoler backend module http request #1349

Conversation

david-heidema
Copy link
Contributor

✔️ Checklist

  • Added tests for new functionality and regression tests for bug fixes
  • Added changeset (run yarn changeset in the root)
  • Screenshots of before and after attached (for UI changes)
  • Added or updated documentation (if applicable)

@david-heidema david-heidema requested a review from a team as a code owner April 18, 2024 16:00
@jwillaz
Copy link

jwillaz commented Apr 18, 2024

#1325

@jwillaz
Copy link

jwillaz commented Apr 23, 2024

@punkle or @Xantier - Any chance one of you has a moment to review these plugin enhancements for the new backend?

Xantier
Xantier previously approved these changes Apr 24, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's name this file something else. It's a bit disingenuous to call this alpha if the plan is to deprecate every other option soon enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Xantier - I went ahead and renamed it "new-backend".

@@ -0,0 +1,5 @@
---
'@roadiehq/scaffolder-backend-module-http-request': patch
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a minor change here

Copy link

@pjungermann pjungermann left a comment

Choose a reason for hiding this comment

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

FYI: I just tried to use it with the new (default) backend system and this does not work as the module is not exported.

I have added suggestions and comments for changes that I believe are necessary to fix it.

I might create a PR for it.

Comment on lines +57 to +58
cd packages/backend
yarn add @roadiehq/scaffolder-backend-module-http-request/alpha

Choose a reason for hiding this comment

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

Suggested change
cd packages/backend
yarn add @roadiehq/scaffolder-backend-module-http-request/alpha
yarn --cwd packages/backend add @roadiehq/scaffolder-backend-module-http-request

const backend = createBackend();
backend.add(import('@backstage/plugin-proxy-backend/alpha'));
backend.add(import('@backstage/plugin-scaffolder-backend/alpha'));
backend.add(import('@roadiehq/scaffolder-backend-module-http-request/alpha'));

Choose a reason for hiding this comment

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

Suggested change
backend.add(import('@roadiehq/scaffolder-backend-module-http-request/alpha'));
backend.add(import('@roadiehq/scaffolder-backend-module-http-request'));

* See the License for the specific language governing permissions and
* limitations under the License.
*/
export { scaffolderBackendModuleHttpRequest as default } from './module';

Choose a reason for hiding this comment

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

This should be moved to the index.ts. Currently, it is not exported for users of the package.

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.

4 participants