You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/05/03 19:13:06 UTC
[GitHub] [accumulo] DomGarguilo opened a new issue, #2673: Enforce formatting of JavaScript files
DomGarguilo opened a new issue, #2673:
URL: https://github.com/apache/accumulo/issues/2673
**Is your feature request related to a problem? Please describe.**
There is no formatting check for JavaScript files and as such much of the code is inconsistent.
**Describe the solution you'd like**
It would be nice to have a formatter for the JavaScript code that is run when the project is built like we have for the java code. We could also add something to the CI checks to ensure files are formatted.
**Describe alternatives you've considered**
* Manually formatting the code with an external tool.
* Leaving the code as is, unformatted and inconsistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo closed issue #2673: Enforce formatting of JavaScript files
Posted by GitBox <gi...@apache.org>.
DomGarguilo closed issue #2673: Enforce formatting of JavaScript files
URL: https://github.com/apache/accumulo/issues/2673
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on issue #2673: Enforce formatting of JavaScript files
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on issue #2673:
URL: https://github.com/apache/accumulo/issues/2673#issuecomment-1151453011
Closed via #2720
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on issue #2673: Enforce formatting of JavaScript files
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #2673:
URL: https://github.com/apache/accumulo/issues/2673#issuecomment-1117943665
> We could minify the code. Then if we need to work on it un-minify and then minify again. There are some tools out there but haven't found anything that we could easily just put in a script.
>
> https://www.npmjs.com/package/minify https://www.npmjs.com/package/unminify
Absolutely not. I would veto that.
1. Minified code does not meet the OSI definition of open source, nor the FSF definition of free software, so we'd have to be careful to only include the minified version in the convenience binary. It should never be used in the source repository or source release artifact. I don't think it's worth even bothering for the binary distribution.
2. Minified code is primarily done for obfuscation and bandwidth savings. Even without browser caching, it's unlikely to have any meaningful effect on bandwidth for our tiny inconsequential javascript files. And, as an open source project, we should explicitly avoid obfuscation.
We previously used to redistribute the minimized version of jQuery. We explicitly [stopped doing that](https://issues.apache.org/jira/browse/ACCUMULO-4741). We definitely shouldn't be doing it for our own files.
Additionally, I would strongly prefer not to depend on anything in NPM/Node. We are not a Javascript project, and we should not depend and that development stack of tools. If it's installed as part of a CI check for code quality, I don't mind. But, I don't want to put that much of a burden on our developers to learn a whole new developer toolset for what is primarily a Java project. Any JS tooling should be quite minimal and extremely convenient to edit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on issue #2673: Enforce formatting of JavaScript files
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #2673:
URL: https://github.com/apache/accumulo/issues/2673#issuecomment-1118501722
> We previously used to redistribute the minimized version of jQuery. We explicitly [stopped doing that](https://issues.apache.org/jira/browse/ACCUMULO-4741). We definitely shouldn't be doing it for our own files.
Someone should give that guy a raise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on issue #2673: Enforce formatting of JavaScript files
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #2673:
URL: https://github.com/apache/accumulo/issues/2673#issuecomment-1118500219
> Minified code does not meet the OSI definition of open source, nor the FSF definition of free software.
Ah, right good call.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on issue #2673: Enforce formatting of JavaScript files
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #2673:
URL: https://github.com/apache/accumulo/issues/2673#issuecomment-1117698105
Just to put this in perspective, we have 18 very small Javascript files (we should not format the `/external/` ones, as those should stay identical to their upstream source, so they match any checksums), vs. over 2000 Java files. Formatting is nice, but where there is a clear benefit to adding a tiny bit of tooling to manage thousands of Java files, the benefit isn't as clear when the tooling for formatting Javascript files is such a larger percentage of the overall Javascript footprint.
That said, if we can get some formatting, with minimal tooling, I'd be in favor of that. I'm currently looking into something similar for our bash scripts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on issue #2673: Enforce formatting of JavaScript files
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #2673:
URL: https://github.com/apache/accumulo/issues/2673#issuecomment-1117863875
We could minify the code. Then if we need to work on it un-minify and then minify again. There are some tools out there but haven't found anything that we could easily just put in a script.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on issue #2673: Enforce formatting of JavaScript files
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #2673:
URL: https://github.com/apache/accumulo/issues/2673#issuecomment-1151479584
> Closed via #2720
Technically, this is not enforced. But, I'm also fine with closing this as "won't fix".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on issue #2673: Enforce formatting of JavaScript files
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #2673:
URL: https://github.com/apache/accumulo/issues/2673#issuecomment-1117189085
If there is push back against introducing a dependency for something automated, maybe we could just have a script to run some CLI tools. I have been running the Node.js package, JSLint to clean up the JS. https://www.jslint.com/help.html
For example, I followed these steps to setup Node.js, install and run JSlint:
1. Install `npm` using: `sudo apt install npm`
2. I ran `mkdir /home/mike/js`
3. `cd /home/mike/js`
4. `npm init` and followed the prompts
5. `npm install jslint`
6. `cd <accumulo_workspace>`
7. `npx jslint server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/bulkImport.js`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org