Skip to content

Conversation

@christianvogt
Copy link
Contributor

@christianvogt christianvogt commented Jul 3, 2019

Adds a new package to the monorepo for knative resources.

  • Adds a Serverless section to the admin nav
  • Contributes three new resource list pages for Services, Revisions, Configurations
    • Pages show the default resource lists without any custom columns
  • Adds feature flag for knative serving resources
  • OWNERS file is just a clone of dev console for now

serverless-admin

cc @joshuawilson @invincibleJai @vojtechszocs

fyi @alimobrem @sspeiche

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 3, 2019
@sspeiche
Copy link

sspeiche commented Jul 3, 2019

@christianvogt Do we plan to put some tech-preview badge / indicator on it?

@spadgett
Copy link
Member

spadgett commented Jul 3, 2019

cc @rebeccaalpert

@spadgett spadgett added this to the v4.2 milestone Jul 3, 2019
@christianvogt
Copy link
Contributor Author

@sspeiche we can add a tech preview badge if needed cc @openshift/team-devconsole-ux

I'd prefer to do any customizations in a separate PR because this PR simply first enables the default pages. We plan to update the pages to be customized lists with custom columns later. We can add the badges there if needed.

@christianvogt
Copy link
Contributor Author

@vojtechszocs i got the following error, so I removed plugin-stats from yarn dev

$ yarn clean && yarn plugin-stats && NODE_ENV=production ts-node -O '{"module":"commonjs"}' ./node_modules/.bin/webpack --mode=production
$ rm -rf ./public/dist
$ ts-node -O '{"module":"commonjs"}' ./plugin-stats.ts
/go/src/github.com/openshift/console/frontend/node_modules/lodash-es/lodash.js:10
export { default as add } from './add.js';
^^^^^^

SyntaxError: Unexpected token export
    at new Script (vm.js:74:7)
    at createScript (vm.js:246:10)
    at Object.runInThisContext (vm.js:298:10)
    at Module._compile (internal/modules/cjs/loader.js:670:28)
    at Module._extensions..js (internal/modules/cjs/loader.js:713:10)
    at Object.require.extensions.(anonymous function) [as .js] (/go/src/github.com/openshift/console/frontend/node_modules/ts-node/src/index.ts:392:14)
    at Module.load (internal/modules/cjs/loader.js:612:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:551:12)
    at Function.Module._load (internal/modules/cjs/loader.js:543:3)
    at Module.require (internal/modules/cjs/loader.js:650:17)

@christianvogt christianvogt force-pushed the knative-plugin branch 5 times, most recently from 7c1f21b to 55ea82c Compare July 3, 2019 21:38
@invincibleJai
Copy link
Member

/test

Copy link
Member

Choose a reason for hiding this comment

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

Shall we move this util as well under knative-plugin, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could. Since the util isn't being used yet and after our discussion I wasn't sure if it was going to be updated to align more closely with the forms apis or not.
For now I'll leave it here and you can move it when you get to completing the work on the import forms.

@christianvogt
Copy link
Contributor Author

rebased

ping @joshuawilson @invincibleJai

Copy link
Member

Choose a reason for hiding this comment

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

am not sure how abbr works but when i try store.getState().k8s.toJS().RESOURCES.models['serving.knative.dev~v1alpha1~KnativeServing'] in browser console registered model has KS as abbr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

@invincibleJai invincibleJai Jul 4, 2019

Choose a reason for hiding this comment

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

labelPlural: 'Knative Servings',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

@invincibleJai invincibleJai left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Member

@invincibleJai invincibleJai left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 4, 2019
@rohitkrai03
Copy link
Contributor

/lgtm

@serenamarie125
Copy link
Contributor

@christianvogt Do we plan to put some tech-preview badge / indicator on it?

@sspeiche @christianvogt yes we want to. We need a convention for OpenShift to do this. We will work with @beanh66 and her team early in the week to propose something and get it reviewed.

@serenamarie125
Copy link
Contributor

FYI @alimobrem

@christianvogt
Copy link
Contributor Author

I'm fine with adding a tech preview badge in some way. But we can do so in another PR once the design is determined.

@spadgett
Copy link
Member

spadgett commented Jul 5, 2019

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, invincibleJai, rohitkrai03, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 5, 2019
@openshift-merge-robot openshift-merge-robot merged commit 5f14b21 into openshift:master Jul 5, 2019
@vojtechszocs
Copy link
Contributor

@vojtechszocs i got the following error, so I removed plugin-stats from yarn dev

I've just rebased & updated #1911 to fix that.

@christianvogt christianvogt deleted the knative-plugin branch August 20, 2020 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants