-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CLOSED] Fix #3094: Refactor JSLint into an extension #2953
Comments
Comment by DennisKehrig Awesome, I'll get to to this later this week! |
Comment by TomMalbran The submodule movement didn't work as I expected and was giving me lots of problem over the last merge. It also didn't made much sense, since this is now an extensions and could be removed. Any other extension uses submodules to load the thirdparty code, so I decided to just remove the submodule and add the files to the branch. I hope that is ok. I fixed the merge issues and fixed a few other things. I also changed the path in the NOTICE. I haven't move it around so that the differences aren't much, but I could do if needed. |
Comment by DennisKehrig Reviewing. |
Comment by DennisKehrig JSLint isn't used by other extensions, it seems, so moving it is not a problem.
|
Comment by DennisKehrig
Please fix the minor issues mentioned above and merge with master again, then I'll run the unit tests again and merge it. Thank you so much! |
Comment by njx Hey there--please hold off on merging this since today is the last day of the sprint and we're trying to close it down. We can merge it next week. Thanks. |
Comment by TomMalbran What I meant before, is that every default extension that uses thirdparty modules, doesn't uses in a submodule way. The files are just included in the thirdparty folder. I haven't checked any other extension before. I also didn't thought that it could be possible to add the submodule directly in the extension folder as the "Continuous compilation did". I will check into that, might be nicer to have the code as a submodule, but defined inside the extension. |
Comment by DennisKehrig This only works for extensions with an own repository, though, so I think you did it exactly right. Especially since many other third party modules like jQuery are just copies, too. |
Comment by TomMalbran Fixed the review comments and merged with master. The main issue I was thinking of when using submodules in an extension is what would happen if the folder doesn't exists, when the extension isn't there. But since is a default extension and removing it would mean to remove it from the repository, it would be needed to remove the submodule too. So I think that re-adding JSLint as a submodule can be good, if you think is needed/better. |
Comment by DennisKehrig Alright, merging! |
Issue by TomMalbran
Friday Mar 15, 2013 at 03:14 GMT
Originally opened as adobe/brackets#3143
This request fixes the issue #3094 by refactoring JSLint into an extension. I also did several changes in JSLintUtils, now being the main file in the extension as a result of the refactor, to use a template for the table and use Language to identify JavaScript files instead of the extension. I was also able to move the JSLint submodule to the extension.
Note 1: I had to touch my git files to move the submodule, so not sure if that will work when merged.
Note 2: Not sure why it didn't renamed JSLintUtils to main and show the differences.
TomMalbran included the following code: https://github.com/adobe/brackets/pull/3143/commits
The text was updated successfully, but these errors were encountered: