12 ms·
Nice! Constructive feedback: Comments! My advice would be to add comments to the code. Comments shouldn't repeat the code, but e.g. describe what it's doing,
by wegs 6y ago
Nice! Constructive feedback:
Comments!
My advice would be to add comments to the code. Comments shouldn't repeat the code, but e.g. describe what it's doing, or provide links to where the formulas come from and what they mean. It's also helpful to document overall code structure (where do I go to find something?).
Formatting!
And to run it through code quality tools. A lot of minor styling issues. It could use more new lines (esp. between functions).
These sorts of things might seem minor, but they matter if you're applying for a job. A lot of people will give a cursory glance at your code repos to see if everything looks reasonable or if there's anything insane. These sorts of surface features make a disproportionate difference.
Plus, they really do help. A lot of your users might not know what LAB colors are, or how exactly HSV is computed.
- semantictags 6y agoThanks for the feedback! Improving the documentation is definitely one of my priorities, but I hadn't thought of documenting the overall structure. Do you have any suggestions for how best to do that?
- wegs 6y agoUsually, in the README, I describe what the major files are. For something this simple, it's usually just one paragraph. "The source code is in `index.js`. In order to build the minified version, run `[command]`. [description of what `index.js.min` is, and how it compares to `dist/ac-colors-min.js`] [Maybe short description of other travis commands used during development]" Basically, imagine you're running into this repo having never looked at it before. What do you want to know? From there, you can go one level deeper, and document `index.js`. It's not rocket science, but it is helpful. For example, I have no idea what `256colors.js` is, or how it's used for testing. That should be either a comment at the top of the file, or in the README (depending on how important the file is). When you go into `index.js`, it's nice to have an overview of what's what in the file, and how it's structured. I like when docs are build from files, actually (so instead of all of this in `README.md`, you have that in comments in code, and build to something like `README.md`). I also like having clearly documented inputs and outputs. I also really recommend blank lines between functions. This is especially true if you have blank lines within functions. lchabString, lchab, and the first part of rgbtoHsl fuse into one big code paragraph right now. I wouldn't necessarily take what I say as how you ought to do it -- that's a question of style, and how I solve these problems. Take it more as a problem statement and example solution.