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: introduce enum programming-language #12052

Merged
merged 5 commits into from Oct 13, 2021

Conversation

pret-a-porter
Copy link
Contributor

Changes:

Added enum ProgrammingLanguage instead of list of constant variables

Context:

This change was requested to be done in separate PR #11690

Documentation

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

I think we should deprecate language from managers, as we now have manager with multiple languages.
I think that can confuse users because they expect language config to apply to them partially.

Golang = 'golang',
Java = 'java',
JavaScript = 'js',
NET = 'dotnet',
Copy link
Member

Choose a reason for hiding this comment

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

🤔 correct spelling would be .NET, should we use DotNet instead?

@rarkins @JamieMagee WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but the brand guidelines are have it as Dotnet.

lib/manager/index.ts Show resolved Hide resolved
Java = 'java',
JavaScript = 'js',
NET = 'dotnet',
NodeJS = 'node',
Copy link
Member

Choose a reason for hiding this comment

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

This is technically no programming language.

I think we should deprecate language from managers.

@rarkins @JamieMagee WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it is not programming language, but ProgrammingLanguage variable name is more descriptive than Language. I open for good suggestions

@rarkins
Copy link
Collaborator

rarkins commented Oct 9, 2021

Let's discuss in an issue. Maybe I already had one, because I am also uncomfortable with how it works today. The biggest challenge is how to keep backwards compatible behavior for those using it. Another possibility is adding config warnings for a while saying "stop using languages please" and hope the app users all change? :)

But for now should we do this refactoring?

@viceice
Copy link
Member

viceice commented Oct 9, 2021

Let's discuss in an issue. Maybe I already had one, because I am also uncomfortable with how it works today. The biggest challenge is how to keep backwards compatible behavior for those using it. Another possibility is adding config warnings for a while saying "stop using languages please" and hope the app users all change? :)

#4165

But for now should we do this refactoring?

Yes, let's do it, as it's mostly done and don't harm.

@rarkins rarkins enabled auto-merge (squash) October 13, 2021 04:41
@rarkins rarkins merged commit a2ceff0 into renovatebot:main Oct 13, 2021
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 28.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2021
@pret-a-porter pret-a-porter deleted the refactor/programming_language branch January 11, 2022 20:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants