Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Update CodeMirror #12613

Merged
merged 1 commit into from
Jul 26, 2016
Merged

Update CodeMirror #12613

merged 1 commit into from
Jul 26, 2016

Conversation

marcelgerber
Copy link
Contributor

See #12609.
cc @ficristo
I ran Unit, Integration and Extension tests without new failures.

@ficristo
Copy link
Collaborator

It seems to me you have used the commit of the CodeMirror repository instead of using the adobe/CodeMirro2: I am unable to update the submodule.
In future can you write in the commit message the version we are going to update? Thank you.

@marcelgerber
Copy link
Contributor Author

Ah, sorry, I forgot to push the updated version to our CodeMirror2 fork. I've done so now so you can try again.

@marcelgerber
Copy link
Contributor Author

I have pulled the test fixes in, so you can have another look.

@ficristo
Copy link
Collaborator

Sorry but I don't think I'll have much time this or the next week to review this PR.
I saw a couple of fixes about backtips on JS scripts on latest master of CodeMirror: I would really like to take them. People is doing more and more ES6 stuff.

So I see at least two options:

  • land this as is. I think CodeMirror is stable enough that if doesn't introduce new errors in the Brackets test suite should good to go. The 1.7 release was released recentish enough that I think we have enough time to verify eventual regressions. (And take another upgrade next months)
  • wait the next release and meanwhile continue to test this PR and fixing the current tests failures

/cc @petetnt @MiguelCastillo @zaggino if they have more thoughts.
(I don't know how were tested the previous versions so having other opinions is good regardless)

@marcelgerber
Copy link
Contributor Author

Just so you know, we usually update to some commit of CodeMirror, not necessarily to a release version (in-between versions of CodeMirror are usually stable as well), so we can take in these recent fixes without problems.
Our testing process is mostly just running all the unit tests, merging, and then looking out for regressions in the issue tracker.

@zaggino
Copy link
Contributor

zaggino commented Jul 25, 2016

👍 for merge

@ficristo
Copy link
Collaborator

ficristo commented Jul 26, 2016

Ok, if you update against latest CodeMirror master and tests pass (or at least doesn't introduce new failures) then is fine by me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants