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.

Sign up for GitHub

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

Jump to bottom

add platform team definition of done #59993

Merged
merged 5 commits into from
Apr 16, 2020
Merged

add platform team definition of done #59993

merged 5 commits into from
Apr 16, 2020

Conversation

mshustov
Copy link
Contributor

[skip-ci]

A draft of Definition of done for the platform team. Feedback and improvements are welcome.
Unfortunately, it's not so easy to add several PR templates for the repo, so having the document is just fine.

@mshustov mshustov requested a review from a team as a code owner March 12, 2020 10:49
@mshustov mshustov added backport:skip This commit does not require backporting chore Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Mar 12, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

pgayvallet
pgayvallet reviewed Mar 20, 2020
View reviewed changes
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

  • Is that a DOD for core or for all our codeowned code?

  • Should we add any of thoses?

    • All the impacted code is in Typescript (mostly thinking about migrating features here, so not sure if relevant)
    • The feature's code and structure follows our code/naming conventions

src/core/CONVENTIONS.md
## Organisational Conventions
### Definition of done
Definition of done for a feature:
- has a dedicated Github issue describing problem space
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't that more in the 'Definition of Ready'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, do we want to have a separate document? We can keep it here for the time being.

Copy link
Contributor

Choose a reason for hiding this comment

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

Keeping it there for now is fine.

src/core/CONVENTIONS.md Outdated
- for a logged-in user
- for anonymous user
- compatible with Spaces
- can be used by the first & third party plugins
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: remove the?

@mshustov
Copy link
Contributor Author

Is that a DOD for core or for all our codeowned code?

That's a high-level convention applicable to all our code.

All the impacted code is in Typescript (mostly thinking about migrating features here, so not sure if relevant)

We can state it, as well a declare our approach to adding new / evaluating existing dependencies.

  • all our tests use standard Kibana tooling - jest & FTR.
  • there shouldn't be several versions of a package in the code.

@mshustov mshustov added v7.8.0 and removed v7.7.0 labels Mar 24, 2020
joshdover
joshdover reviewed Mar 25, 2020
View reviewed changes
Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

A couple more suggestions:

  • has appropriate logging on the server-side
  • has been verified manually by at least one reviewer

@mshustov
Copy link
Contributor Author

mshustov commented Apr 1, 2020 •
edited
Loading

has appropriate logging on the server-side

that sounds confusing. we don't have a definition of appropriate logging 😅 I think we should develop recommendations (what, when, how) for platform & plugins, but it's a separate task.

has been verified manually by at least one reviewer

👍

joshdover
joshdover reviewed Apr 15, 2020
View reviewed changes
Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

that sounds confusing. we don't have a definition of appropriate logging 😅 I think we should develop recommendations (what, when, how) for platform & plugins, but it's a separate task.

👍

joshdover
joshdover approved these changes Apr 15, 2020
View reviewed changes
Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

Let's go ahead and get this in

@mshustov mshustov merged commit f18aea5 into elastic:master Apr 16, 2020
@mshustov mshustov deleted the platform-dod branch April 16, 2020 09:53
@mshustov mshustov removed the backport:skip This commit does not require backporting label Apr 16, 2020
@mshustov mshustov mentioned this pull request Apr 16, 2020
Merged
mshustov added a commit to mshustov/kibana that referenced this pull request Apr 16, 2020
@mshustov
add platform team definition of done (elastic#59993)
6be97fb 
* add platform team definition of done

* add link to ToC

* add suggestions from Josh

* add tooling

* add testing note
mshustov added a commit that referenced this pull request Apr 16, 2020
@mshustov
add platform team definition of done (#59993) (#63676)
773c25d 
* add platform team definition of done

* add link to ToC

* add suggestions from Josh

* add tooling

* add testing note
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 17, 2020
@gmmorris
Merge branch 'master' into alerting/es-query
08062cf 
* master: (56 commits)
  [i18n] Update CODEOWNERS (elastic#63354)
  add platform team definition of done (elastic#59993)
  [SIEM] move away from Joi for importing/exporting timeline (elastic#62125)
  Fix discover preserve url (elastic#63580)
  [alerting] Adds an alertServices mock and uses it in siem, monitoring and uptime (elastic#63489)
  Closes elastic#63109 for Service Map by resetting edges styles for the selected node (elastic#63655)
  MIgrated index_header to react (elastic#63490)
  Index pattern management UI -> TypeScript and New Platform Ready (indexed_fields_table) (elastic#63364)
  [SIEM] [Cases] Insert timeline and reporters/tags in table bug fixes (elastic#63642)
  [Reporting] Make usable default element positions (elastic#63191)
  [Reporting] Switch Serverside Config Wrapper to NP (elastic#62500)
  [Reporting] Add "warning" status as an alternate type of completed job (elastic#63498)
  Split action types into own page (elastic#63516)
  [Lens] Only show copy on save for previously saved docs (elastic#63535)
  Update README.md (elastic#63622)
  Bugfix clear saved query crashes kibana on Discover in some cases (elastic#63554)
  Add uptime CODEOWNER entries. (elastic#63616)
  [ML] Extract apiDoc params from the schema definitions (elastic#62933)
  Fix alerting documentation encryption key requirement (elastic#63512)
  Fix CODEOWNERS and sass lint paths (elastic#63552)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshdover joshdover joshdover approved these changes

@rudolf rudolf Awaiting requested review from rudolf

@pgayvallet pgayvallet Awaiting requested review from pgayvallet

Assignees
No one assigned
Labels
chore Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.8.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mshustov @elasticmachine @rudolf @pgayvallet @joshdover