Deprecations

[jessitron]

FOR DISCUSSION NOT YET MERGE

Objective: Deprecate methods that could be implemented in TypeScript on top of a few primitives that form the TS-Scala Rug interface. Primitives are:

• Project:
  • name
  • list files
  • add file
  • delete file
  • move file (not currently there)
  • find file
  • fileExists
  • directoryExists
• Directory:
  • name
  • path
  • list files (not currently there)
  • set name (not currently there)
  • set path (not currently there)
  • find file
  • fileExists
  • directoryExists
• File:
  • name
  • path
  • content
  • set content
  • set name
  • set path
  • contains (this is just so useful in tests)

Proposal: We deprecate these methods in the next rug milestone, and add the ones marked as TODO in the changes. (Discussion before implementation)

Then before the following one, we update any rugs that use them. We'll find out whether we really did want them that badly. Then we remove them.

Comments on these primitives???

[jessitron]

@kipz good point.

The next step in this process is to provide TypeScript replacements. We should do that before deprecating, so we can point people to them.

On Fri, May 26, 2017 at 6:42 AM, David Dooling notifications@github.com wrote:

@ddgenome commented on this pull request.

Looks good, if a bit scary. Definitely needs a change log entry. Perhaps a fancy find and replace could be used to add @deprecated Scaladoc comment pointing to this PR.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/atomist/rug/pull/622#pullrequestreview-40502154, or mute the thread https://github.com/notifications/unsubscribe-auth/ABGLKdx3BzwtcmzKFZv6yy7hzZXfvAY7ks5r9rq_gaJpZM4NnH_P .

--

• jessitron@atomist.com -

[johnsonr]

This is important: much cleaner and it dramatically increases the potential for unit testing.

In parallel with it, there is something else important we need to discuss in rationalizing this interface, in the same timeframe. It could be a separate issue, but it's intimately linked so may be we should handle it here.

Currently we assume that people are going to work with strings on files, in TypeScript. This isn't the right approach if we encounter very large files. A Java InputStream is available on FileArtifact--whether it's more efficient obviously depends on the implementation--but there's no way to get at it in TypeScript. The new microgrammar support allows transparently working against an InputStream (minimal TypeScript abstraction), and we should allow that style of interaction in TypeScript in general.

This would mean:

• Add a streaming abstraction in File. We need to consider how to drive close. Maybe it is exposed only in a callback. It could simply have the same methods as the TypeScript InputStream in the microgrammar project.
• Ensuring that File.contains, which is indeed useful, isn't implemented to fully materialize a string in all cases.
• (Less urgent) Add an offset-oriented or streaming-oriented update method, so we don't need to materialize the whole string for that either.

This may involve ArtifactSource

[johnsonr]

I think File.replace is worth keeping. As it could adopt a streaming implementation without the need to go to the TypeScript layer. Not, however, the regex methods, as JavaScript support is different (and syntactically better).

[johnsonr]

I think we can add a streaming implementation in a separate PR, and we should progress this one first.

[jessitron]

performance is a valid reason to make primitives of things that are not inherently primitive.

On Mon, Jul 10, 2017 at 2:43 AM, johnsonr notifications@github.com wrote:

@johnsonr commented on this pull request.

In src/main/scala/com/atomist/rug/kind/core/ProjectMutableView.scala https://github.com/atomist/rug/pull/622#discussion_r126352735:

@@ -99,9 +100,11 @@ class ProjectMutableView( @ExportFunction(readOnly = true, exposeAsProperty = true, description = "The total number of files in this project")

• @Deprecated

Why is this deprecated? I think this is arguably a primitive because it may be more efficient

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/atomist/rug/pull/622#pullrequestreview-48826978, or mute the thread https://github.com/notifications/unsubscribe-auth/ABGLKcKd2m2t50EYp0VrttWkou3Dn9Nfks5sMdYwgaJpZM4NnH_P .

--

• jessitron@atomist.com -

[ddgenome]

No longer relevant.