Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

move code to src folder and switch to commonjs #4851

Merged
merged 9 commits into from
Aug 4, 2022
Merged

move code to src folder and switch to commonjs #4851

merged 9 commits into from
Aug 4, 2022

Conversation

nightwing
Copy link
Member

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@nightwing nightwing force-pushed the commonjs branch 6 times, most recently from 48972c4 to a0b78c6 Compare July 25, 2022 09:21
@codecov
Copy link

codecov bot commented Jul 25, 2022

Codecov Report

Merging #4851 (2398568) into master (c2c749b) will increase coverage by 15.67%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #4851       +/-   ##
===========================================
+ Coverage   70.07%   85.74%   +15.67%     
===========================================
  Files         557      539       -18     
  Lines       58221    40602    -17619     
  Branches    11195     6419     -4776     
===========================================
- Hits        40796    34814     -5982     
+ Misses      17425     5788    -11637     
Flag Coverage Δ
unittests 85.74% <ø> (+15.67%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/ace/mode/coffee/coffee.js
lib/ace/mode/coffee/parser_test.js
lib/ace/mode/css/csslint.js
lib/ace/mode/javascript/jshint.js
lib/ace/mode/jsoniq.js
lib/ace/tokenizer_test.js
lib/ace/mode/mysql_highlight_rules.js
lib/ace/mode/latte_highlight_rules.js
lib/ace/mode/nginx_highlight_rules.js
lib/ace/mode/vala.js
... and 827 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2c749b...2398568. Read the comment docs.

@nightwing nightwing changed the title [WIP] move code to src and switch to commonjs move code to src folder and switch to commonjs Jul 28, 2022
@nightwing nightwing merged commit 416ae9e into master Aug 4, 2022
@nightwing nightwing deleted the commonjs branch August 4, 2022 09:11
lildude added a commit to github-linguist/linguist that referenced this pull request Aug 8, 2022
The directory structure was changed in ajaxorg/ace#4851
@lildude
Copy link

lildude commented Aug 9, 2022

@nightwing @andrewnester 👋 from github/linguist, the library responsible for language detection on github.com, and other places.

This PR has come to my attention as one of our tests has started failing because of this change. GitHub no longer uses the Ace editor (it hasn't for quite some time) however we maintain a check against Ace modes for all languages added for third-party users of the gem that may still be using Ace.

A change on our side is easy enough to implement (change the path), but I'm curious to know what the rationale is for this change as it's not explained in the OP and there's no referencing issue.

Also, were https://github.com/ajaxorg/ace/blob/master/lib/ace/mode/jsoniq.js and https://github.com/ajaxorg/ace/blob/master/lib/ace/mode/xquery.js deliberately left in lib/ace/mode and not moved to src/mode?

Having these two files in the old location makes our change a little more complicated as we now need to query two locations. I'm also trying to judge if things are likely to change again within the near future.

@andrewnester
Copy link
Contributor

HI @lildude ! Thanks for reaching out!
The idea of the change comes from the fact that we want to publish a source code of Ace as an NPM package. To make it developer friendly we decided to switch to Common JS first so hence this change.

The mentioned modes are deliberately left in the lib folders :)

lildude added a commit to github-linguist/linguist that referenced this pull request Aug 13, 2022
* Update ace mode path

The directory structure was changed in ajaxorg/ace#4851

* Grab both locations
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants