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

Linter should check for SDK_VERSION in code and package.json #15643

Closed
YoDaMa opened this issue Jun 9, 2021 · 5 comments
Closed

Linter should check for SDK_VERSION in code and package.json #15643

YoDaMa opened this issue Jun 9, 2021 · 5 comments
Labels
EngSys This issue is impacting the engineering system. eslint plugin

Comments

@YoDaMa
Copy link
Contributor

YoDaMa commented Jun 9, 2021

The automated PR for incrementing version number after release. For the @azure/iot-modelsrepository package, I created a test that would validate that the version number in package.json incremented in lockstep with the version number in my contants. Because the automated PR only updates package.json, this PR will fail and cause problems. See:

#15464

The problem is in the modelsrepository code I did not define the constant SDK_VERSION and it was not declared in the package.json file. The linter should catch this and raise an error, since this is the proper way to define the SDK_VERSION for constants.

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 9, 2021
@YoDaMa
Copy link
Contributor Author

YoDaMa commented Jun 9, 2021

cc @ramya-rao-a

@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 9, 2021
@ramya-rao-a ramya-rao-a added the EngSys This issue is impacting the engineering system. label Jun 9, 2021
@ramya-rao-a ramya-rao-a added this to the Backlog milestone Jun 9, 2021
@ramya-rao-a
Copy link
Contributor

Thanks @YoDaMa

@deyaaeldeen Do we have any checks like this today? If not, can you share some details we can go about doing this?

@deyaaeldeen
Copy link
Member

Hey @YoDaMa

The bot can increment the src/constants.ts file as well, e.g. #15620. I think you need to set the constants entry in the metadata entry in package.json for this to work.

Regarding lint checking versions, we do not have a check for version consistency between package.json and src/constants.ts. In order to make such a check, we need to parse package.json as a JS object and pass it (or just the version bit) to a new rule that checks src/constants.ts. This is related to something @witemple-msft and I talked about, which is basically simplifying lint-checking json files like package.json by doing simple parsing and checking instead of the pattern-matching approach used currently in the linter. This could speed things a bit.

@ramya-rao-a
Copy link
Contributor

I was thinking more of making sure the linter checks for the constants entry in the metadata entry in the package.json file

@ramya-rao-a
Copy link
Contributor

In the latest version of the code generator, we will be forming the user agent string in the generated code and there would be no need to have a separate SDK_VERSION defined in the convenience layer. See #15777 for example. So this issue would no longer be applicable. Closing.

@xirzec xirzec removed this from the Backlog milestone May 17, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
EngSys This issue is impacting the engineering system. eslint plugin
Projects
None yet
Development

No branches or pull requests

4 participants