Skip to content
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

Read app path from applications file #13

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

JekRock
Copy link

@JekRock JekRock commented Mar 1, 2023

Addresses #9

Change ECMAScript version in eslint config to pass linting.
ECMAScript 2020 doesn't include private class fields that this project
is using
.eslintrc.json Outdated
@@ -6,7 +6,7 @@
},
"extends": "eslint:recommended",
"parserOptions": {
"ecmaVersion": 2020
"ecmaVersion": 2022
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"ecmaVersion": 2022
"ecmaVersion": "latest"

main.js Outdated
Comment on lines 3 to 4
const { resolve } = require('node:path');
const { readFile } = require('node:fs/promises');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to have compound identifiers like path.resolve in codebase below

main.js Outdated
Comment on lines 17 to 21
const appsFile = await readFile(resolve(appFilePath), {
encoding: 'utf8',
});

return appsFile.split('\n').filter((path) => path.trim() !== '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const appsFile = await readFile(resolve(appFilePath), {
encoding: 'utf8',
});
return appsFile.split('\n').filter((path) => path.trim() !== '');
const appsFile = path.resolve('.applications')
const apps = await readFile(appsFile, 'utf8');
return apps.split('[\r\n\s]+').filter((s) => s.length !== 0);

main.js Outdated

const { Logger, StreamForLogger } = require('./src/logger.js');
const http = require('./src/http.js');
const ws = require('./src/ws.js');
const { loadApplication } = require('./src/loader.js');

const APPLICATION_PATH = path.join(process.cwd(), '../NodeJS-Application');
const APPLICATIONS_FILE_PATH = '.applications';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need no constant, string content is easy to understand

main.js Outdated

return appsFile.split('\n').filter((path) => path.trim() !== '');
};

(async () => {
const streamForLogger = new StreamForLogger(LOG_FOLDER_PATH);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const streamForLogger = new StreamForLogger(LOG_FOLDER_PATH);
const stream = new StreamForLogger(LOG_FOLDER_PATH);

main.js Outdated
Comment on lines 26 to 28
const server = fastify({
logger: { level: 'info', stream: streamForLogger },
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const server = fastify({
logger: { level: 'info', stream: streamForLogger },
});
const server = fastify({ logger: { level: 'info', stream } });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants