Skip to content

Commit

Permalink
chore: tag todo comments with additional info
Browse files Browse the repository at this point in the history
Make them full sentences.
Indent wrapped lines.
List the person blamed for adding it.
Update some dates.
  • Loading branch information
lishaduck committed Jul 8, 2024
1 parent db26edf commit bc949b8
Show file tree
Hide file tree
Showing 16 changed files with 46 additions and 46 deletions.
26 changes: 13 additions & 13 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// @ts-check
// TODO [eslint@>9.5]: Use `.ts` extension to get more type checking for this file.
// TODO [engine:node@>=18]: Upgrade `tseslint`.
// TODO [engine:node@>=18]: Use `eslint-define-config` to get type checking for this file.
// TODO [engine:node@>=18]: Use `eslint-plugin-jsdoc` to get JSDoc linting.
// TODO(@lishaduck) [eslint@>9.5]: Use `.ts` extension to get more type checking for this file.
// TODO(@lishaduck) [engine:node@>=18]: Upgrade `tseslint`.

Check warning on line 3 in .eslintrc.js

View workflow job for this annotation

GitHub Actions / test

There is a TODO match for Node.js version: node>=18. Upgrade `tseslint`
// TODO(@lishaduck) [engine:node@>=18]: Use `eslint-define-config` to get type checking for this file.

Check warning on line 4 in .eslintrc.js

View workflow job for this annotation

GitHub Actions / test

There is a TODO match for Node.js version: node>=18. Use `eslint-define-config` to get type checking for this file
// TODO(@lishaduck) [engine:node@>=18]: Use `eslint-plugin-jsdoc` to get JSDoc linting.

Check warning on line 5 in .eslintrc.js

View workflow job for this annotation

GitHub Actions / test

There is a TODO match for Node.js version: node>=18. Use `eslint-plugin-jsdoc` to get JSDoc linting

module.exports = {
root: true,
Expand Down Expand Up @@ -67,7 +67,7 @@ module.exports = {
'@typescript-eslint/switch-exhaustiveness-check': 'error',
'@typescript-eslint/consistent-type-definitions': ['error', 'type'],
'default-case': 'off',
'n/shebang': 'off', // TODO [eslint-plugin-n@>=17]: Turn on 'n/hashbang'. For now, `shebang` is buggy.
'n/shebang': 'off', // TODO(@lishaduck) [eslint-plugin-n@>=17]: Turn on 'n/hashbang'. For now, `shebang` is buggy.
'@eslint-community/eslint-comments/require-description': 'error',
strict: ['error', 'global'],
'unicorn/import-style': [
Expand All @@ -84,35 +84,35 @@ module.exports = {
'unicorn/prevent-abbreviations': 'off',
'no-fallthrough': 'off', // TS checks for this, and TSESLint doesn't provide an alternative.

// TODO: Once there are no more `any`s, start enforcing these rules.
// TODO(@lishaduck): Once there are no more `any`s, start enforcing these rules.
'@typescript-eslint/no-unsafe-assignment': 'off',
'@typescript-eslint/no-unsafe-argument': 'off',
'@typescript-eslint/no-unsafe-member-access': 'off',
'@typescript-eslint/no-unsafe-call': 'off',
'@typescript-eslint/no-unsafe-return': 'off',

// TODO: Enable stricter promise rules.
// TODO(@lishaduck): Enable stricter promise rules.
'@typescript-eslint/no-misused-promises': 'off',
'@typescript-eslint/no-floating-promises': 'off',
'promise/catch-or-return': 'off',
'promise/always-return': 'off',

// TODO: Security issues that should eventually get fixed.
// TODO(@lishaduck): Security issues that should eventually get fixed.
'security/detect-object-injection': 'off',
'security/detect-non-literal-fs-filename': 'off',
'security/detect-non-literal-require': 'off',
'security/detect-unsafe-regex': 'off', // TODO: Add `eslint-plugin-regexp` and fix these issues.
'security/detect-unsafe-regex': 'off', // TODO(@lishaduck): Add `eslint-plugin-regexp` and fix these issues.

// TODO: Enable rules that require newer versions of Node.js when we bump the minimum version.
'unicorn/prefer-string-replace-all': 'off', // TODO [engine:node@>=15]: Enable this rule.
'unicorn/prefer-at': 'off' // TODO [engine:node@>=16.6]: Enable this rule.
// TODO(@lishaduck): Enable rules that require newer versions of Node.js when we bump the minimum version.
'unicorn/prefer-string-replace-all': 'off', // TODO(@lishaduck) [engine:node@>=15]: Enable this rule.

Check warning on line 107 in .eslintrc.js

View workflow job for this annotation

GitHub Actions / test

There is a TODO match for Node.js version: node>=15. Enable this rule
'unicorn/prefer-at': 'off' // TODO(@lishaduck) [engine:node@>=16.6]: Enable this rule.

Check warning on line 108 in .eslintrc.js

View workflow job for this annotation

GitHub Actions / test

There is a TODO match for Node.js version: node>=16.6. Enable this rule
},
overrides: [
{
files: ['./new-package/**/*.js'],
rules: {
'n/no-process-exit': 'off',
'@typescript-eslint/unbound-method': 'off' // TODO: Fix this warning. @lishaduck just got confused.
'@typescript-eslint/unbound-method': 'off' // TODO(@lishaduck): Fix this warning. I just got confused.
}
},
{
Expand Down
3 changes: 1 addition & 2 deletions bin/elm-review
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#!/usr/bin/env node

// TODO: With ESM, this should use TLA.

// TODO(@lishaduck): With ESM, this should use TLA.
require('../lib/main')();
6 changes: 3 additions & 3 deletions lib/autofix.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function askConfirmationToFixWithOptions(options, app, elmVersion) {
filesToFix.map((file) => {
const filePath = path.resolve(basePath, file.path);
if (filePath.endsWith('.elm')) {
// TODO Make this async
// TODO(@jfmengels): Make this asynchronous.
return formatFileContent(options, file, filePath);
}

Expand All @@ -120,8 +120,8 @@ function askConfirmationToFixWithOptions(options, app, elmVersion) {

let dependencies;
if (modifiedElmJson) {
// TODO Check if source-directories have changed, and if so, load/unload the necessary files.
// TODO Check if dependencies have changed before we do this
// TODO(@jfmengels): Check if source-directories have changed, and if so, load/unload the necessary files.
// TODO(@jfmengels): Check if dependencies have changed before we do this.
dependencies = await ProjectDependencies.collect(
options,
JSON.parse(modifiedElmJson.source),
Expand Down
6 changes: 3 additions & 3 deletions lib/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ function unique(array) {
return [...new Set(array)];
}

// TODO: TSify
// TODO(@lishaduck): TSify these type definitions.
/**
* @typedef {object} CompileOptions
* @property {string} cwd
Expand Down Expand Up @@ -490,7 +490,7 @@ function compilationError(options, stderr) {
console.log(stderr);
}

// TODO Handle this better
// TODO(@jfmengels): Handle this better.

exit(1);
}
Expand Down Expand Up @@ -579,7 +579,7 @@ async function buildElmParser(options, reviewElmJson) {
),
// Needed when the user has `"type": "module"` in their package.json.
// Our output is CommonJS.
// TODO: Switch to a .cjs file instead.
// TODO(@lishaduck): Switch to a .cjs file instead.
FS.mkdirp(options.generatedCodePackageJson()).then(() =>
FS.writeFile(
path.join(options.generatedCodePackageJson(), 'package.json'),
Expand Down
4 changes: 2 additions & 2 deletions lib/elm-binary.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ function downloadDependenciesOfElmJson(elmBinary, elmJsonPath) {

if (result.status !== 0) {
const error = JSON.parse(result.stderr.toString());
// TODO Check for other kinds of errors
// TODO(@jfmengels): Check for other kinds of errors.
if (error.title !== 'NO INPUT') {
// TODO Print error nicely
// TODO(@jfmengels): Print the error nicely.
throw new Error(error);
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/elm-files.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ async function getProjectFiles(options, elmSyntaxVersion) {
({files}) => files.length === 0
);

// TODO Have a different message depending on how directories were chosen (CLI directories, application, package)
// TODO(@jfmengels): Have a different message depending on how directories were chosen (CLI directories, application, package).
if (options.directoriesToAnalyze.length > 0 && emptyDirectories.length > 0) {
throw new ErrorMessage.CustomError(
'NO FILES FOUND',
Expand Down
3 changes: 1 addition & 2 deletions lib/error-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,7 @@ function formatJson(debug, error, defaultPath) {
type: 'error',
title: error.title,
path: error.path ?? defaultPath,
// TODO We currently strip the colors, but it would be nice to keep them
// so that editors can have nicer error messages
// TODO(@jfmengels): We currently strip the colors, but it would be nice to keep them so that editors can have nicer error messages.
message: [stripAnsi(error.message.trim())],
stack: debug && error.stack ? stripAnsi(error.stack) : undefined
};
Expand Down
2 changes: 1 addition & 1 deletion lib/init.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const fs = require('graceful-fs');
const path = require('node:path');
// TODO [engine:node@>=16.7.0]: Use native `cp`.
// TODO(@lishaduck) [engine:node@>=16.7.0]: Use native `cp`.

Check warning on line 3 in lib/init.js

View workflow job for this annotation

GitHub Actions / test

There is a TODO match for Node.js version: node>=16.7.0. Use native `cp`
const fsExtra = require('fs-extra');
const chalk = require('chalk');
const prompts = require('prompts');
Expand Down
2 changes: 1 addition & 1 deletion lib/module-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const Cache = require('./cache');
*/
function subscribe(options, app) {
AppState.subscribe(app.ports.cacheFile, ({source, ast}) =>
// TODO Do this in a thread
// TODO(@jfmengels): Do this work in a thread.
cacheFile(options, Hash.hash(source), ast)
);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/new-package.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const path = require('node:path');
const childProcess = require('node:child_process');
const chalk = require('chalk');
const prompts = require('prompts');
// TODO [engine:node@>=16.7.0]: Use native `cp`.
// TODO(@lishaduck) [engine:node@>=16.7.0]: Use native `cp`.

Check warning on line 5 in lib/new-package.js

View workflow job for this annotation

GitHub Actions / test

There is a TODO match for Node.js version: node>=16.7.0. Use native `cp`
const fsExtra = require('fs-extra');
const Init = require('./init');
const Spinner = require('./spinner');
Expand Down
6 changes: 3 additions & 3 deletions lib/project-json-files.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ async function getElmJson(options, elmVersion, name, packageVersion) {
}
}

// TODO Empty this cache at some point?
// Note that we might need it in watch and fix mode, but not otherwise.
// TODO(@jfmengels): Empty this cache at some point?
// Note that we might need it in watch and fix mode, but not otherwise.
/** @type {Map<string, Promise<PackageElmJson>>} */
const elmJsonInElmHomePromises = new Map();

Expand Down Expand Up @@ -177,7 +177,7 @@ async function readFromPackagesWebsite(
packageVersion,
file
) {
// TODO [engine:node@>=18]: We can use `fetch` now.
// TODO(@lishaduck) [engine:node@>=21]: We can use `fetch` now.
const response = await got(
`https://package.elm-lang.org/packages/${packageName}/${packageVersion}/${file}`
);
Expand Down
4 changes: 2 additions & 2 deletions lib/remote-template.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ async function downloadTemplateElmJson(template, commit) {
const {repoName, pathToFolder} = template;
const pathToFolderAsUrl = pathToFolder ? `/${pathToFolder}` : '';

// TODO [engine:node@>=18]: We can use `fetch` now.
// TODO(@lishaduck) [engine:node@>=21]: We can use `fetch` now.
const response = await got(
`https://raw.githubusercontent.com/${repoName}/${commit}${pathToFolderAsUrl}/elm.json`
).catch((error) => {
Expand Down Expand Up @@ -270,7 +270,7 @@ async function makeGitHubApiRequest(options, url, handleNotFound) {

Debug.log(`Making API request to GitHub: ${url}`);
try {
// TODO [engine:node@>=18]: We can use `fetch` now.
// TODO(@lishaduck) [engine:node@>=21]: We can use `fetch` now.
const response = await got(url, parameters);

return response.body;
Expand Down
2 changes: 1 addition & 1 deletion lib/result-cache-json.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function replacer(_key, value) {
}

if (typeof value === 'object') {
// TODO Serialize Dicts also
// TODO (@jfmengels): Also support serializing `Dict`s.
// if (value.$ === -1) {
// return {
// $: '$D',
Expand Down
7 changes: 4 additions & 3 deletions lib/result-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ let worker = null;
const resultCache = new Map();
const promisesToResolve = new Map();

// TODO Create an alternate version of Options used in elm-app-worker.
// Depending on whether this is running in a worker or not, the options will be slightly different.
// TODO(@jfmengels): Create an alternate version of Options used in
// `elm-app-worker`. Depending on whether this is running in a worker or not,
// the options will be slightly different.
/**
* Load cache results.
*
Expand All @@ -30,7 +31,7 @@ const promisesToResolve = new Map();
* @returns {Promise<void>}
*/
async function load(options, ignoredDirs, ignoredFiles, cacheFolder) {
// TODO Make caching work for all of these
// TODO(@jfmengels): Make caching work for all of these combinations.
if (
options.debug ||
options.directoriesToAnalyze.length > 0 ||
Expand Down
15 changes: 8 additions & 7 deletions lib/watch.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const path = require('node:path');
// TODO [engine:node@>=19.1] Use `fs.watch` (or `@parcel/watcher`).
// TODO(@lishaduck) [engine:node@>=19.1]: Use `fs.watch` (or `@parcel/watcher`).
const chokidar = require('chokidar');
const Debug = require('./debug');
const FS = require('./fs-wrapper');
Expand Down Expand Up @@ -94,8 +94,8 @@ function watchFiles(
clearConsole();
}

// TODO Detect what has changed and only re-load the necessary parts.
// We do some of this work in `autofix.js` already.
// TODO(@jfmengels): Detect what has changed and only re-load the necessary parts.
// We do some of this work in `autofix.js` already.
Debug.log('Your `elm.json` has changed. Restarting elm-review.');
}

Expand Down Expand Up @@ -438,16 +438,17 @@ function createExtraFilesWatcher(options, app, runReview, onError, request) {
*/
function createSuppressedFilesWatcher(options, app, onError) {
function updateSuppressedErrors() {
// TODO Write last save time for each of these in appstate, and compare with the last update time
// that is given as argument to this function. If possible, don't do anything.
// TODO(@jfmengels): Write last save time for each of these in appstate,
// and compare with the last update time that is given as argument to
// this function. If possible, don't do anything.
if (suppressedErrorsTimeout) {
clearTimeout(suppressedErrorsTimeout);
}

suppressedErrorsTimeout = setTimeout(async () => {
const suppressedErrors = await SuppressedErrors.read(options);
// TODO Avoid doing anything if suppressed errors haven't changed
// It's likely this program's fault for changing anything anyway
// TODO(@jfmengels): Avoid doing anything if suppressed errors haven't
// changed. It's likely this program's fault for changing anything anyway.
Debug.log('Suppressed errors have been added');
app.ports.updateSuppressedErrors.send(suppressedErrors);
}, 20);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"check-engines": "ls-engines --mode=ideal",
"elm-format": "elm-format --validate ast-codec init-templates new-package parseElm review template",
"elm-tests": "(cd template/ && elm make src/Elm/Review/Main.elm --output=/dev/null && elm-test)",
"eslint-check": "eslint . --report-unused-disable-directives --max-warnings=10",
"eslint-check": "eslint . --report-unused-disable-directives --max-warnings=7",
"eslint-fix": "npm run eslint-check -- --fix",
"jest": "npx jest",
"prepare": "elm-tooling install",
Expand Down

0 comments on commit bc949b8

Please sign in to comment.