package Git LFS, and also add linux support #45

Merged
merged 28 commits into from Nov 3, 2016

Conversation

Projects
None yet
3 participants
Owner

shiftkey commented Oct 25, 2016

Fixes #12
Fixes #47

This introduces some Typescript to slipstream Git LFS into the vanilla Git packages. I've also added a Ubuntu build to start testing things.

  • build and package a new v2.10.1 release for Windows with Git LFS support
  • build and package a new v2.10.1 release for macOS with Git LFS support
  • build and package a new v2.10.1 release for Ubuntu with Git LFS support
  • rewrite PR to drop token :rage4:
  • add Ubuntu tests to Travis
  • update download-git.js to support Ubuntu CI (use vanilla package for now)
  • make Travis do one build, not two :rage3:
  • cleanup tracing

@shiftkey shiftkey changed the title from [WIP] package Git LFS, cut a new release to [WIP] package Git LFS, and also add linux support Nov 1, 2016

@shiftkey shiftkey closed this Nov 1, 2016

@shiftkey shiftkey reopened this Nov 1, 2016

shiftkey added some commits Nov 1, 2016

@shiftkey shiftkey changed the title from [WIP] package Git LFS, and also add linux support to package Git LFS, and also add linux support Nov 1, 2016

Owner

shiftkey commented Nov 1, 2016

There's a bunch of tidy up I need to do inside the packaging flow, but I'll open issues for that after this lands.

Owner

shiftkey commented Nov 2, 2016

Given all the pain I had with finding a reliable unzip operation for both zip and tar.gz files, I might look at porting this to a bash script and use the native tools. But this is working at least...

@joshaber joshaber self-assigned this Nov 2, 2016

Looks like a fun headache 👍

.travis.yml
- osx
-
+
@joshaber

joshaber Nov 2, 2016

Owner

I like that whitespace. That's a nice whitespace.

@shiftkey

shiftkey Nov 2, 2016

Owner

Yuuuuuge whitespace

branches:
only:
- master
language: node_js
node_js:
- - "node"
+ - "6"
@joshaber

joshaber Nov 2, 2016

Owner

Good call 👍

script/download-git.js
- })
+function extract (source, callback) {
+ if (path.extname(source) === '.zip') {
+ // console.log('extracting zip file')
@joshaber

joshaber Nov 2, 2016

Owner

Can we remove this?

@shiftkey

shiftkey Nov 2, 2016

Owner

Yeah, I'll scrub those

script/download-git.js
+ })
+
+ } else {
+ // console.log('extracting tgz file')
@joshaber

joshaber Nov 2, 2016

Owner

🔥

script/functions.ts
+ }
+}
+
+export class FileOperations {
@joshaber

joshaber Nov 2, 2016

Owner

It seems like this file could do with a little breaking up. Maybe Config/FileOperations/LFS/Downloader or something?

script/functions.ts
+ // strip any leading directory information
+ strip: 1,
+ // only extract a given set of file extensions
+ filter: (file: { path: string} ) => path.extname(file.path) === extension
@joshaber

joshaber Nov 2, 2016

Owner

Nit: space between string and }

script/functions.ts
+ if (platform === 'win32') {
+ const nestedPath = path.join(destination, 'mingw64', 'libexec', 'git-core')
+ return Archiver.extractAndFlatten(source, nestedPath, '.exe')
+ } else if (platform === 'darwin') {
@joshaber

joshaber Nov 2, 2016

Owner

It looks like we can unify the Darwin and Linux branches.

script/functions.ts
+ return new Promise((resolve, reject) => {
+ checksum.file(file, { algorithm: 'sha256' }, (err: Error, hash: string) => {
+
+ if (err) {
@joshaber

joshaber Nov 2, 2016

Owner

Indentation is a bit scribbly here.

script/functions.ts
+}
+
+export class Downloader {
+ private static getReleaseAssets = (owner: string, repo: string, tag: string): Promise<ReadonlyArray<Asset>>=> {
@joshaber

joshaber Nov 2, 2016

Owner

Is there a reason these functions are defined as class properties, instead of being class functions?

@joshaber

joshaber Nov 2, 2016

Owner
>>=

For a hot second there I thought we had some monad action

@shiftkey

shiftkey Nov 2, 2016

Owner

.img hot monad action

@shiftkey

shiftkey Nov 2, 2016

Owner

But seriously, no real reason here. I've heard functions are pretty great, though...

script/package.ts
+}
+
+if (process.argv.length < 2) {
+ console.log('node ./package.js [win32|darwin]')
@joshaber

joshaber Nov 2, 2016

Owner

Should this include linux now?

shiftkey added some commits Nov 2, 2016

Owner

shiftkey commented Nov 3, 2016

🍎

@joshaber joshaber merged commit b346b15 into master Nov 3, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@joshaber joshaber deleted the package-with-git-lfs branch Nov 3, 2016

kuychaco commented Nov 9, 2016

oops, almost forgot to celebrate with this:

Thanks @shiftkey! You'll make many Atom users on Linux very happy :)

@BinaryMuse BinaryMuse referenced this pull request in atom/github Jan 10, 2017

Closed

Linux support #297

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment