这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@wawhal
Copy link
Contributor

@wawhal wawhal commented Nov 27, 2018

fix #489
Tests are WIP. Opening the PR for review.

Description

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

Solution and Design

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@hasura-bot
Copy link
Contributor

Review app for commit 2df117b deployed to Heroku: https://hge-ci-pull-1124.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1124-2df117b

@shahidhk
Copy link
Member

This UI needs overhauling to match with other tabs. like settings -> modify etc.

@hasura-bot
Copy link
Contributor

Review app for commit 7755b53 deployed to Heroku: https://hge-ci-pull-1124.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1124-7755b53

@shahidhk shahidhk added the c/console Related to console label Nov 29, 2018
@wawhal wawhal added the s/do-not-merge Do not merge this pull request to master label Nov 29, 2018
@hasura-bot
Copy link
Contributor

Review app for commit 70da1b6 deployed to Heroku: https://hge-ci-pull-1124.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1124-70da1b6

@hasura-bot
Copy link
Contributor

Review app for commit cbf4637 deployed to Heroku: https://hge-ci-pull-1124.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1124-cbf4637

Copy link
Contributor

@tirumaraiselvan tirumaraiselvan left a comment

Choose a reason for hiding this comment

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

LGTM

@shahidhk
Copy link
Member

@wawhal Can we show the headers in a table format instead of the JSON?

@wawhal wawhal added s/ok-to-merge Status: This pull request can be merged to master and removed s/do-not-merge Do not merge this pull request to master labels Dec 14, 2018
@wawhal
Copy link
Contributor Author

wawhal commented Dec 14, 2018

@shahidhk tried that. Doesn't look good when there are more headers.

@hasura-bot
Copy link
Contributor

Review app for commit 297261f deployed to Heroku: https://hge-ci-pull-1124.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1124-297261f

Copy link
Contributor

@karthikvt26 karthikvt26 left a comment

Choose a reason for hiding this comment

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

@wawhal toggle checkbox when clicked on the labels too

@wawhal wawhal removed the request for review from praveenweb December 15, 2018 05:31
@hasura-bot
Copy link
Contributor

Review app for commit d1164e5 deployed to Heroku: https://hge-ci-pull-1124.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1124-d1164e5

karthikvt26
karthikvt26 previously approved these changes Dec 15, 2018
Copy link
Contributor

@karthikvt26 karthikvt26 left a comment

Choose a reason for hiding this comment

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

LGTM

@shahidhk shahidhk changed the title enable updating event trigger from console enable updating event trigger from console (close #489) Dec 16, 2018
@hasura-bot
Copy link
Contributor

Review app for commit f167548 deployed to Heroku: https://hge-ci-pull-1124.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1124-f167548

@shahidhk shahidhk requested a review from dsandip December 17, 2018 06:47
Copy link
Member

@dsandip dsandip left a comment

Choose a reason for hiding this comment

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

Would like one tiny change: Add Delete this trigger and create another one if you need to edit any of these fields. to the tooltip next to Info.

@hasura-bot
Copy link
Contributor

Review app for commit 96ad349 deployed to Heroku: https://hge-ci-pull-1124.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1124-96ad349

@shahidhk shahidhk merged commit 9d8ac86 into hasura:master Dec 17, 2018
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-1124.herokuapp.com is deleted

hasura-bot pushed a commit that referenced this pull request Sep 19, 2024
<!-- The PR description should answer 2 important questions: -->

### What

To avoid lifetime issues we are wrapping types we reuse throughout
`graphql_ir` and `execute::plan` in `Arc`. This PR does not remove the
later lifetimes, just does the first mechanical step in
`metadata_resolve`

### How

Wrap `ModelSource` and `CommandSource` in `Arc`, fix type errors.
Functional no-op.

V3_GIT_ORIGIN_REV_ID: af91ec7eedf9fe5c3c2fe9737f9c827c1f826d63
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/console Related to console s/ok-to-merge Status: This pull request can be merged to master

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add event trigger update ui to console

8 participants