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

Adding suppliers showing integration with S/4 #196

Draft
wants to merge 112 commits into
base: main
Choose a base branch
from
Draft

Conversation

danjoa
Copy link
Member

@danjoa danjoa commented Feb 17, 2021

No description provided.

@danjoa danjoa requested a review from chgeo February 17, 2021 12:24
Copy link
Member

@chgeo chgeo left a comment

Choose a reason for hiding this comment

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

Cool to see David's DKOM code and spaceflight coming back.

That's still quite something to digest and deserves a lot of explaining, e.g how to juggle the different launch scenarios and processes . Need to explain that in the consumption guide.

@danjoa danjoa marked this pull request as draft February 17, 2021 18:02
@danjoa
Copy link
Member Author

danjoa commented Feb 17, 2021

Still work in progress ... :)
Partly because it uses some convenient shortcuts we just added to be published with upcoming release, like this one:

class DatabaseService {
  ...
  /** Convenience shortcut to check whether an entity instance exists */
  async exists (entity,key) {
     return this.run (SELECT(1).from(entity,key))
  }
}

Rationale for that one: I'm afraid many would just do something like this:

let exists = db.read('Books',201)
if (exists) ...

But this would do a SELECT * which might read tens or hundreds of element data unnecessarily

@danjoa danjoa marked this pull request as ready for review April 10, 2021 11:06
@danjoa danjoa marked this pull request as draft April 10, 2021 11:07
@CLAassistant
Copy link

CLAassistant commented Jun 9, 2021

CLA assistant check
All committers have signed the CLA.

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.

6 participants