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

Refactor: Dedicated Location For All The Constants #5088

Closed
2 of 7 tasks
souravdasslg opened this issue Jan 2, 2020 · 6 comments · Fixed by #5167
Closed
2 of 7 tasks

Refactor: Dedicated Location For All The Constants #5088

souravdasslg opened this issue Jan 2, 2020 · 6 comments · Fixed by #5167
Assignees
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others

Comments

@souravdasslg
Copy link
Contributor

souravdasslg commented Jan 2, 2020

What would you like Renovate to be able to do?
Throughout the renovate codebase, a lot of constant is used, e.g. error type, error code, Http status code and etc. A better approach is to move them to a specific place. That will help code more readable and maintainable.

Describe the solution you'd like
Creating a directory named constants and creating files according to their category. Each file will export constants. Descriptions will be added to understand the constant if required.

Additional context
Here is the category of constants

  • Errors
  • HTTP Status Codes
  • Languages
  • Config Stages
  • Updates Type
  • Validation Constants
  • Migration constants eg. removed options
@rarkins
Copy link
Collaborator

rarkins commented Jan 2, 2020

Let's start with the custom error codes we throw, e.g. repository-changed or registry-failure. Should we put each group into an extra file (e.g. lib/constraints/error-codes.ts) or combined into one file like lib/util/constants.ts? Any opinions @JamieMagee @viceice ?

@rarkins rarkins added needs-requirements priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others labels Jan 2, 2020
@viceice
Copy link
Member

viceice commented Jan 2, 2020

I think multiple files are more readable. I think we have a lot constants. Maybe we can use typescript enum's for better compiletime checks.

Another option is to use literal types like:

export type RepositoryStatus = 'repository-changed' | 'other-state';

@rarkins
Copy link
Collaborator

rarkins commented Jan 3, 2020

Let's start with any X that we new like new Error(X) currently. They can be defined in lib/constants/error-messages.ts.

@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 19.102.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@viceice
Copy link
Member

viceice commented Jan 17, 2020

not yet finished

@souravdasslg
Copy link
Contributor Author

@rarkins @viceice Can you please help to point out what should we do next?
Should we go for HTTP statuses?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants