Skip to content

Update modelforge documentation - #30

Merged
vmarkovtsev merged 1 commit into
src-d:masterfrom
r0mainK:feature/readme
Jun 22, 2018
Merged

vmarkovtsev merged 1 commit into
src-d:masterfrom
r0mainK:feature/readme

Conversation

@r0mainK

@r0mainK r0mainK commented Jun 5, 2018

Copy link
Copy Markdown
Contributor

Last split, simply an update to the doc.

@r0mainK r0mainK changed the title Update readme Update readme [part 5] Jun 5, 2018
@r0mainK r0mainK changed the title Update readme [part 5] Update modelforge documentation Jun 19, 2018
@r0mainK

r0mainK commented Jun 19, 2018

Copy link
Copy Markdown
Contributor Author

EDIT: there will probably be changes -- regarding naming -- from #29

So anyway I put separated doc for commands/API in two files, and modified a bit the README.

@zurk zurk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just several remarks, not fully reviewed.

Comment thread doc/Makefile
@@ -1,25 +0,0 @@
# Minimal makefile for Sphinx documentation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you sure we do not need the files you deleted anymore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, was waiting for @vmarkovtsev 's input on this. I have no idea what is the purpose of these, although since they were not modified since july of last year I don't imagine they are important.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the legacy of src-d/docsrv and can be removed

Comment thread doc/api.md
@@ -0,0 +1,50 @@
# API usage

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please add a link to API usage file to the README.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it exists

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, missed it.

Comment thread doc/cmd.md
@@ -0,0 +1,161 @@
# Command line usage

@zurk zurk Jun 19, 2018 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please add a link to CLI usage to the README

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it exists see Usage sub section.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, missed it.

@r0mainK

r0mainK commented Jun 20, 2018

Copy link
Copy Markdown
Contributor Author

Rebased off #29

Comment thread README.md Outdated
VENDOR = "source{d}" # name of the issuing vendor for models
BACKEND = "gcs" # type of backend to use
BACKEND_ARGS = "bucket='models.cdn.sourced.tech',credentials='key.json'" # all backend arguments
DEFAULT_REPO = "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/src-d/models" # git repo for the index

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ok this looks ugly, I should not have changed. Let's rename to INDEX_REPO and CACHE_DIR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

aha ok

Comment thread README.md
python3 -m unittest discover /path/to/modelforge
```

If you wish to make your model available in [src-d/models](https://github.com/src-d/models), please

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We want to remove any source{d} branding. Change src-d to user everywhere and explain that it is an example.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

got it np

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

btw apart from all the other branding in cmd.md and readme.md that i removed, do you also want this part out ? since in the contributions section i thought it would be ok to leave it as is

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes. We can refer to src-d/models as an example but the code snippets should be neutral.

Comment thread doc/api.md Outdated
inheriting from the `StorageBackend` class in `storage_backend.py`, and define the following
methods:

- `initialize_backend(self, force: bool)`: should initialize the backend. An error should be raised if

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We renamed it to reset()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Signed-off-by: Romain Keramitas <r.keramitas@gmail.com>
@marnovo

marnovo commented Jun 21, 2018

Copy link
Copy Markdown
Member

Since this PR is about documentation, I'd add @campoy as a reviewer.

@vmarkovtsev

Copy link
Copy Markdown
Collaborator

I think it is better to post-review here, otherwise we will wait much time without any updated docs at all.

@vmarkovtsev
vmarkovtsev merged commit 12cf374 into src-d:master Jun 22, 2018
@r0mainK
r0mainK deleted the feature/readme branch June 22, 2018 08:41
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.

4 participants