Skip to content

feat: adding typescript and new imports to the project#112

Open
YoelFerreyra wants to merge 5 commits intomainfrom
feat/adding-typescript-to-the-project
Open

feat: adding typescript and new imports to the project#112
YoelFerreyra wants to merge 5 commits intomainfrom
feat/adding-typescript-to-the-project

Conversation

@YoelFerreyra
Copy link
Collaborator

No description provided.

@YoelFerreyra YoelFerreyra self-assigned this Feb 6, 2025
src/index.ts Outdated
client.commands = new Collection();
deployCommands(client);
deployEvents(client);
//deployEvents(client);
Copy link
Contributor

@heliomar-pena heliomar-pena Feb 6, 2025

Choose a reason for hiding this comment

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

Did you miss to uncomment this function?

}

function handleCriticalError(error) {
function handleCriticalError(error: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe Error is a better interface for this:

Suggested change
function handleCriticalError(error: any) {
function handleCriticalError(error: Error) {

Copy link
Contributor

Choose a reason for hiding this comment

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

please take a look at this comment

import path from 'path';

const logDir = path.join(__dirname, '../log');
const logDir = path.join(import.meta.url, '../log'); // Cambié __dirname por import.meta.url, ya que en módulos ES no tienes acceso a __dirname
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this comment and add it to the PR description instead

Copy link
Contributor

@heliomar-pena heliomar-pena left a comment

Choose a reason for hiding this comment

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

Uf great work man!

dotenv.config();
import { parseAllowedChannels } from './csv-parser-allowed-channels.ts'
import path from 'path'
import type { CronScheduleReview, DiscordServer, GeminiIntegration, MappedStatusCommands, PrTemplate, QAMention, RequestPoint, ScheduleCalendar, ScheduleMessages, TimeZone, VotePoints } from './types/config.d.js';
Copy link
Contributor

Choose a reason for hiding this comment

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

creo que los archivos de definicion de tipos deben ser .ts y no .js

};

interface ParsedSchedule {
variables: Record<string, string>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think here you are defining parameters of parseMarkdownSchedule, and variables is not a parameter of this function

* @param client - Client object.
*/
const processMarkdownFiles = (client) => {
export const processMarkdownFiles = (client: unknown): void => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Discord doesn't have a type definition for client?

.filter((file) => file.endsWith('.js'));
for (const file of commandFiles) {
const command = require(path.join(commandsPath, file));
const command = await import(path.join(commandsPath, file));
Copy link
Contributor

Choose a reason for hiding this comment

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

Sometimes dynamic import give problems in the build, have you tried the build to see if there is not error?

} else {
console.log(
`[WARNING] The command at ${filePath} is missing a required "data" or "execute" property.`
`[WARNING] The command at is missing a required "data" or "execute" property.`
Copy link
Contributor

Choose a reason for hiding this comment

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

missed file path in the error message

@@ -0,0 +1,73 @@
import { REST, Routes, Client } from 'discord.js';
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is the Client discord's type, you can use it in here: https://github.com/TeamNovaSoft/novabot/pull/112/files#r1949021235

import { processMarkdownFiles } from './cron/utils/read-markdown-messages.ts';
import { scheduleReviewCheck } from './cron/schedule-code-review.ts';


Copy link
Contributor

Choose a reason for hiding this comment

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

extra space can be removed

Suggested change

Comment on lines +7 to +10
const translationsPath: string = path.join(
path.dirname(new URL(import.meta.url).pathname),
'translations'
);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we could use process.cwd() instead of path.dirname(new URL(import.meta.url).pathname),?, I am not sure we should test it

@Israel-Laguan
Copy link
Contributor

Check for rebase

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.

3 participants