Skip to content

Fixes #39548 - Fix lint tests, and clean up js code#617

Open
MariaAga wants to merge 2 commits into
theforeman:masterfrom
MariaAga:fix-lint-test
Open

Fixes #39548 - Fix lint tests, and clean up js code#617
MariaAga wants to merge 2 commits into
theforeman:masterfrom
MariaAga:fix-lint-test

Conversation

@MariaAga

@MariaAga MariaAga commented Jul 21, 2026

Copy link
Copy Markdown
Member

While fixing the lint errors, I notice that some come from un-imported/unused files, so deleted them instead.
Lint fix for theforeman/foreman#11008 theforeman/foreman#11098 theforeman/foreman#10968

const EXPONENTIAL_THRESHOLD = 1e21;
const CLAMP_SCALE_FACTOR = 0.9;
const HOURS_PER_HALF_DAY = 12;
const MINUTES_PER_HOUR = 60;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Eh, do we really really have to declare a constant saying that there are 60 minutes in an hour in every single plugin we have? I mean I get where it's coming from, but it feels a bit silly

@MariaAga MariaAga Jul 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I mostly dont want someone to delete a small constant from core because they think no one uses them, and cause much damage. It seems like a simple way to ensure that. As 1 hour is a fact generally, its not like core decides what is an hour. (unlike status names for example)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@adamruzicka I had same question, and now I think that it will be safer do redefine it in plugins.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I can't say I'm convinced.

While the argument of "what if we delete the constant from core" is appealing from the developer's point of view, it also sounds a lot like "but then we won't be able to freely change what is essentially a public interface".

As 1 hour is a fact generally, its not like core decides what is an hour. (unlike status names for example)

This goes both ways. Neither foreman nor plugins decide what http 401 means, yet this one gets imported, while the others do not.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I totally agree with @adamruzicka.
Just had a look, there are some libraries like https://github.com/date-fns/date-fns/blob/4098115cf705e3af7f663d8e5b0686e39a9f478a/pkgs/core/src/constants/index.ts#L133 - but it would be another npm package we need to maintain...

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.

5 participants