thlorenz / brace

📔 browserify compatible version of the ace editor.
http://thlorenz.github.io/brace/
MIT License
1.06k stars 304 forks source link

Brace 0.11.2 (bumps ace to 1.4.2; adds travis support) #152

Closed StoneCypher closed 5 years ago

StoneCypher commented 5 years ago

This PR:

  1. Bumps the underlying ACE implementation
  2. Adds support for Travis-CI assuming to check Node 8-11
djMax commented 5 years ago

I would definitely love to see brace updated to the latest ace, it's tough to diagnose issues when the underlying dep is so far behind. react-ace is also suffering as a result and it'd be a lot easier (in theory) to fix their problem here than having them move off brace.

StoneCypher commented 5 years ago

I pinged him on Twitter and he said he'd see to this mid-January

So I think this might be coming soon?

StoneCypher commented 5 years ago

@thlorenz Hey buddy, could you please hit merge?

thlorenz commented 5 years ago

Hey looks good @StoneCypher , but I'd prefer two things:

  1. Don't bump the brace version .. I do this as part of the publish
  2. Provide the add travis part as a separate PR

So ideally this PR would have ONE commit which bumps the version in the update script and includes the result of running it (changed Ace files).

StoneCypher commented 5 years ago

i don't understand what you mean by "bumps the version in the update script"

thlorenz commented 5 years ago

I mean please don't change the brace version inside the package.json as I take care of that during the publish step to npm. As in please remove this commit (along with the other ones unrelated to the upgrade).

StoneCypher commented 5 years ago

I have removed the .travis.yml and regressed the version in package.json.

I believe that's what I'm supposed to do, but I have a bad flu and I'm stupid when I'm sick

StoneCypher commented 5 years ago

I will reintegrate travis in a followup PR if you say this is what you meant

thlorenz commented 5 years ago

OK, could you please squash all this into one commit? Basically all we need for this PR are the changes in this commit.

At this point there are tons of commits here including a merge of another PR which I'm not sure about why it is even here. Alternatively open a new PR with just the upgrade related commit and we'll go from there. Thanks.

StoneCypher commented 5 years ago

Oh. Okay

StoneCypher commented 5 years ago

ahaaaa haaaaaaaaaaaa downloading large repos on conference wifi

StoneCypher commented 5 years ago

https://github.com/thlorenz/brace/pull/153