Skip to content

Commit

Permalink
Add ESLint rules to limit comment width (#1613)
Browse files Browse the repository at this point in the history
## Motivation for the change, related issues

Long doc comments are hard to read and it's easy to write long comments.

We need an automated way of checking for the comment length. 

## Implementation details

This PR adds
[eslint-plugin-comment-length](https://www.npmjs.com/package/eslint-plugin-comment-length?activeTab=readme)
to implement comment max-len `eslint` rules.

The errors aren't fixed to avoid creating issues with this PR, but they
will show up as warnings, so we can fix them in the future.

## Testing Instructions (or ideally a Blueprint)

- CI
  • Loading branch information
bgrgicak authored Aug 6, 2024
1 parent 4587f68 commit 27dc040
Show file tree
Hide file tree
Showing 59 changed files with 490 additions and 237 deletions.
27 changes: 26 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"overrides": [
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"extends": ["plugin:comment-length/recommended"],
"rules": {
"@nx/enforce-module-boundaries": [
"error",
Expand Down Expand Up @@ -34,7 +35,31 @@
"no-constant-condition": 0,
"no-nested-ternary": 0,
"jsx-a11y/click-events-have-key-events": 0,
"jsx-a11y/no-static-element-interactions": 0
"jsx-a11y/no-static-element-interactions": 0,
"comment-length/limit-single-line-comments": [
"warn",
{
"maxLength": 100,
"tabSize": 0,
"ignoreUrls": true
}
],
"comment-length/limit-multi-line-comments": [
"warn",
{
"maxLength": 100,
"tabSize": 0,
"ignoreUrls": true
}
],
"comment-length/limit-tagged-template-literal-comments": [
"warn",
{
"maxLength": 100,
"tabSize": 0,
"ignoreUrls": true
}
]
}
},
{
Expand Down
162 changes: 161 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@
"esbuild-plugin-ignore": "1.1.1",
"eslint": "8.46.0",
"eslint-config-prettier": "8.1.0",
"eslint-plugin-comment-length": "1.7.3",
"eslint-plugin-cypress": "^2.13.4",
"eslint-plugin-import": "2.27.5",
"eslint-plugin-jsx-a11y": "6.7.1",
Expand Down
4 changes: 2 additions & 2 deletions packages/php-wasm/cli/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ async function run() {
' '
)} ${process.argv[1]}`;
// Naively replace the PHP binary with the PHP-WASM command
// @TODO: Don't process the command. Lean on the shell to do it, e.g. through
// a PATH or an alias.
// @TODO: Don't process the command. Lean on the shell to do it, e.g.
// through a PATH or an alias.
const updatedCommand = command.replace(
/^(?:\\ |[^ ])*php\d?(\s|$)/,
phpWasmCommand + '$1'
Expand Down
19 changes: 10 additions & 9 deletions packages/php-wasm/fs-journal/src/lib/fs-journal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,8 @@ export function normalizeFilesystemOperations(
latter.operation === 'RENAME' &&
former.operation === 'RENAME'
) {
// Normalizing a double rename is a complex scenario so let's just give up.
// There's just too many possible scenarios to handle.
// Normalizing a double rename is a complex scenario so let's just give
// up. There's just too many possible scenarios to handle.
//
// For example, the following scenario may not be possible to normalize:
// RENAME /dir_a /dir_b
Expand All @@ -395,8 +395,8 @@ export function normalizeFilesystemOperations(
// CREATE_DIR /dir_b
// CREATE_FILE /dir_b/file_2
//
// But that's not a straightforward transformation so let's just not handle
// it for now.
// But that's not a straightforward transformation so let's just not
// handle it for now.
logger.warn(
'[FS Journal] Normalizing a double rename is not yet supported:',
{
Expand All @@ -410,8 +410,8 @@ export function normalizeFilesystemOperations(
if (former.operation === 'CREATE' || former.operation === 'WRITE') {
if (latter.operation === 'RENAME') {
if (formerType === 'same_node') {
// Creating a node and then renaming it is equivalent to creating it in
// the new location.
// Creating a node and then renaming it is equivalent to creating
// it in the new location.
substitutions[j] = [];
substitutions[i] = [
{
Expand All @@ -421,8 +421,8 @@ export function normalizeFilesystemOperations(
...(substitutions[i] || []),
];
} else if (formerType === 'descendant') {
// Creating a node and then renaming its parent directory is equivalent
// to creating it in the new location.
// Creating a node and then renaming its parent directory is
// equivalent to creating it in the new location.
substitutions[j] = [];
substitutions[i] = [
{
Expand All @@ -446,7 +446,8 @@ export function normalizeFilesystemOperations(
latter.operation === 'DELETE' &&
formerType === 'same_node'
) {
// Creating a node and then deleting it is equivalent to doing nothing.
// Creating a node and then deleting it is equivalent to doing
// nothing.
substitutions[j] = [];
substitutions[i] = [];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

/**
* **Call this inside a service worker.**
* These errors include Playground errors like Asyncify errors. PHP errors won't trigger this event.
* These errors include Playground errors like Asyncify errors. PHP errors
* won't trigger this event.
*
* Reports service worker metrics.
* Allows the logger to request metrics from the service worker by sending a message.
* The service worker will respond with the number of open Playground tabs.
* Allows the logger to request metrics from the service worker by sending a
* message. The service worker will respond with the number of open Playground
* tabs.
*
* @param worker The service worker
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ const addServiceWorkerMessageListener = (loggerInstance: Logger) => {
if (event.data?.numberOfOpenPlaygroundTabs === undefined) {
return;
}
// Each tab sends an activate event on load. Prevent sending the same metrics multiple times if a tab is reloaded.
// Each tab sends an activate event on load. Prevent sending the same
// metrics multiple times if a tab is reloaded.
if (
numberOfOpenPlaygroundTabs ===
event.data?.numberOfOpenPlaygroundTabs
Expand Down
3 changes: 2 additions & 1 deletion packages/php-wasm/logger/src/lib/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ export const formatLogEntry = (
/**
* Add a listener for the Playground crashes.
* These crashes include Playground errors like Asyncify errors.
* The callback function will receive an Event object with logs in the detail property.
* The callback function will receive an Event object with logs in the detail
* property.
*
* @param loggerInstance The logger instance
* @param callback The callback function
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ async function onWsConnect(client: any, request: http.IncomingMessage) {
clientLog('resolved ' + reqTargetHost + ' -> ' + reqTargetIp);
} catch (e) {
clientLog("can't resolve " + reqTargetHost + ' due to:', e);
// Send empty binary data to notify requester that connection was initiated
// Send empty binary data to notify requester that connection was
// initiated
client.send([]);
client.close(3000);
return;
Expand Down
20 changes: 11 additions & 9 deletions packages/php-wasm/progress/src/lib/progress-tracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,18 @@ export interface ProgressReceiver {
const PROGRESS_EPSILON = 0.00001;

/**
* The ProgressTracker class is a tool for tracking progress in an operation that is
* divided into multiple stages. It allows you to create sub-trackers for each stage,
* with individual weights and captions. The main tracker automatically calculates the
* progress based on the weighted sum of each sub-tracker's progress. This makes it easy
* to keep track of a complex, multi-stage process and report progress in a user-friendly way.
* The ProgressTracker class is a tool for tracking progress in an operation
* that is divided into multiple stages. It allows you to create sub-trackers
* for each stage, with individual weights and captions. The main tracker
* automatically calculates the progress based on the weighted sum of each
* sub-tracker's progress. This makes it easy to keep track of a complex,
* multi-stage process and report progress in a user-friendly way.
*
* After creating the sub-trackers, you can call the set() method to update the progress
* of the current stage. You can also call the finish() method to mark the current stage
* as complete and move on to the next one. Alternatively, you can call the fillSlowly()
* method to simulate progress filling up slowly to 100% before calling finish().
* After creating the sub-trackers, you can call the set() method to update the
* progress of the current stage. You can also call the finish() method to mark
* the current stage as complete and move on to the next one. Alternatively,
* you can call the fillSlowly() method to simulate progress filling up slowly
* to 100% before calling finish().
*
* @example
* ```ts
Expand Down
Loading

0 comments on commit 27dc040

Please sign in to comment.