Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[master < E-meta] Add MAGE meta module docs #1017

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

imilinovic
Copy link
Contributor

@imilinovic imilinovic commented Aug 31, 2023

Description

Add MAGE meta module docs

Pull request type

Please delete options that are not relevant and check the ones that are.

  • New documentation page

Related PRs and issues

PR this doc page is related to:
PR296
Closes (link to issue):

Checklist:

  • I checked all content with Grammarly
  • I performed a self-review of my code
  • I made corresponding changes to the rest of the documentation
  • The build passes locally
  • My changes generate no new warnings or errors

@imilinovic imilinovic added the status: draft PR is in draft phase label Aug 31, 2023
@imilinovic imilinovic self-assigned this Aug 31, 2023
@imilinovic imilinovic marked this pull request as ready for review August 31, 2023 11:55
@imilinovic imilinovic added status: ready PR is ready for review and removed status: draft PR is in draft phase labels Aug 31, 2023

### Procedures

## stats
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
## stats
## `stats()`

If it's a procedure?

Copy link
Contributor Author

@imilinovic imilinovic Aug 31, 2023

Choose a reason for hiding this comment

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

Stats is not a procedure but it made sense to me to write about it since stats_online and stats_offline do the same thing (described in stats) so I wrote it as some sort of summary

Comment on lines 54 to 55
- stats_online - works in **O(1)** and requires setting up a trigger
- stats_offline - traverses the whole graph
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- stats_online - works in **O(1)** and requires setting up a trigger
- stats_offline - traverses the whole graph
- `stats_online` - works in **O(1)** and requires setting up a trigger
- `stats_offline` - traverses the whole graph

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you listing the same outputs three times then?
I don't think i would do that... i would just have stats online and offline and then this knowledge

  • stats_online - works in O(1) and requires setting up a trigger
  • stats_offline - traverses the whole graph

can be in the intro, or each in its own section.

It's confusing like this + different from all the other mage pages


### `stats_offline()`

Retrieves the graph metadata by traversing the whole graph. *stats_online* should be preferred because of the better complexity unless you don't want to use triggers.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Retrieves the graph metadata by traversing the whole graph. *stats_online* should be preferred because of the better complexity unless you don't want to use triggers.
Retrieves the graph metadata by traversing the whole graph. `stats_online` should be preferred because of the better complexity unless you don't want to use triggers.

@vpavicic vpavicic added status: change PR reviewed - needs changes and removed status: ready PR is ready for review labels Aug 31, 2023
Copy link
Collaborator

@vpavicic vpavicic left a comment

Choose a reason for hiding this comment

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

i think i would have just online and offline headings, without the summary stats

@imilinovic imilinovic added status: ready PR is ready for review and removed status: change PR reviewed - needs changes labels Sep 11, 2023
@imilinovic imilinovic requested a review from vpavicic September 11, 2023 05:29
@vpavicic vpavicic added status: ship it PR approved and removed status: ready PR is ready for review labels Sep 11, 2023
@vpavicic
Copy link
Collaborator

This was already in the util category

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants