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

shorten examples in all functions #210

Closed
MarcinKosinski opened this issue Feb 7, 2016 · 12 comments
Closed

shorten examples in all functions #210

MarcinKosinski opened this issue Feb 7, 2016 · 12 comments
Assignees

Comments

@MarcinKosinski
Copy link
Collaborator

They are too long and can be created with the usage of Repository that is embed in the package

@pbiecek
Copy link
Owner

pbiecek commented Feb 7, 2016

Good idea, but when?
Is is planned as a pre 2.0 feature?

2016-02-07 22:16 GMT+01:00 Marcin Kosiński notifications@github.com:

They are too long and can be created with the usage of Repository that is
embed in the package


Reply to this email directly or view it on GitHub
#210.

pozdrawiam serdecznie,
Przemysław Biecek

MarcinKosinski added a commit that referenced this issue Feb 7, 2016
@MarcinKosinski
Copy link
Collaborator Author

I am doing it right now

2016-02-07 22:27 GMT+01:00 Przemysław Biecek notifications@github.com:

Good idea, but when?
Is is planned as a pre 2.0 feature?

2016-02-07 22:16 GMT+01:00 Marcin Kosiński notifications@github.com:

They are too long and can be created with the usage of Repository that is
embed in the package


Reply to this email directly or view it on GitHub
#210.

pozdrawiam serdecznie,
Przemysław Biecek


Reply to this email directly or view it on GitHub
#210 (comment).

@pbiecek
Copy link
Owner

pbiecek commented Feb 7, 2016

QL

2016-02-07 22:31 GMT+01:00 Marcin Kosiński notifications@github.com:

I am doing it right now

2016-02-07 22:27 GMT+01:00 Przemysław Biecek notifications@github.com:

Good idea, but when?
Is is planned as a pre 2.0 feature?

2016-02-07 22:16 GMT+01:00 Marcin Kosiński notifications@github.com:

They are too long and can be created with the usage of Repository that
is
embed in the package


Reply to this email directly or view it on GitHub
#210.

pozdrawiam serdecznie,
Przemysław Biecek


Reply to this email directly or view it on GitHub
<#210 (comment)
.


Reply to this email directly or view it on GitHub
#210 (comment).

pozdrawiam serdecznie,
Przemysław Biecek

@MarcinKosinski
Copy link
Collaborator Author

I've shortened few functions. You can also have a look at functions that I

  • didn't move but are not so long
    • aoptions
    • asearch
    • cache
    • copyToRepo
    • getTags
    • setRepo
  • shortened but one can look again at
    • loadFromRepo
  • still long and didn't move
    • removeFromRepo

MarcinKosinski added a commit that referenced this issue Feb 7, 2016
MarcinKosinski added a commit that referenced this issue Feb 7, 2016
@pbiecek
Copy link
Owner

pbiecek commented Feb 7, 2016

For copyToRepo, getTags, setRepo, loadFromRepo, removeFromRepo
I would export and use also Local or Remote versions.

2016-02-07 23:28 GMT+01:00 Marcin Kosiński notifications@github.com:

I've shortened few functions. You can also have a look at functions that I

didn't move but are not so long

  • aoptions

    • asearch
    • cache
    • copyToRepo
    • getTags

    - setRepo

    shortened but one can look again at

  • loadFromRepo

    still long and didn't move

  • removeFromRepo


Reply to this email directly or view it on GitHub
#210 (comment).

pozdrawiam serdecznie,
Przemysław Biecek

@MarcinKosinski
Copy link
Collaborator Author

I Think loadFromRepo does not exist.
setRepo, copyToRepo, getTags are not exported

No idea what to do with removeFromRepo

2016-02-07 23:34 GMT+01:00 Przemysław Biecek notifications@github.com:

For copyToRepo, getTags, setRepo, loadFromRepo, removeFromRepo
I would export and use also Local or Remote versions.

2016-02-07 23:28 GMT+01:00 Marcin Kosiński notifications@github.com:

I've shortened few functions. You can also have a look at functions that
I

didn't move but are not so long

  • aoptions
  • asearch
  • cache
  • copyToRepo
  • getTags

- setRepo

shortened but one can look again at

- loadFromRepo

still long and didn't move

  • removeFromRepo


Reply to this email directly or view it on GitHub
<#210 (comment)
.

pozdrawiam serdecznie,
Przemysław Biecek


Reply to this email directly or view it on GitHub
#210 (comment).

@pbiecek
Copy link
Owner

pbiecek commented Feb 8, 2016

So I am waining for this one and will start #211

@pbiecek
Copy link
Owner

pbiecek commented Feb 8, 2016

@MarcinKosinski deprecated functions, like createEmptyRepo() are in these long examples.

@MarcinKosinski
Copy link
Collaborator Author

Ok I understand. What do we do about rmFromRepo ? changing the name to rmFromLocalRepo?

MarcinKosinski added a commit that referenced this issue Feb 8, 2016
MarcinKosinski added a commit that referenced this issue Feb 8, 2016
@MarcinKosinski
Copy link
Collaborator Author

I have verified longer examples and removed deprecated names.
The only issue we have is with rmFromRepo. I'll change this here #218

@MarcinKosinski
Copy link
Collaborator Author

I've checked examples. Changed delete/create/rmFrom name convention to maintain consistency with Local/Remote function names. I also moved dontrun in few examples to make sure the package and examples work. I think examples are now run in about 80% of manual pages.

pbiecek added a commit that referenced this issue Feb 8, 2016
# By MarcinKosinski
# Via MarcinKosinski
* 'master' of https://github.com/pbiecek/archivist:
  udpate NEWS.md for #210 #218
  check examples after close #218
@pbiecek
Copy link
Owner

pbiecek commented Feb 8, 2016

I will move rmFromRepo to rmFromLocalRepo.
The only question is, should the rmFromRepo be deprecated / non visible / just a copy

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

No branches or pull requests

2 participants