-
Notifications
You must be signed in to change notification settings - Fork 30.5k
[openui5] Update definition files for OpenUI5 1.90 #53680
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
Conversation
|
@akudev Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through. This is a live comment which I will keep updated. 1 package in this PR
Code ReviewsThere aren't any other owners of this package, so a DT maintainer will review it. You can test the changes in this PR in the Playground. Status
All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 53680,
"author": "akudev",
"headCommitOid": "a1963e29036e9392666531c6a16e5b184759a446",
"lastPushDate": "2021-06-08T13:20:49.000Z",
"lastActivityDate": "2021-06-08T20:02:58.000Z",
"mergeOfferDate": "2021-06-08T19:33:02.000Z",
"mergeRequestDate": "2021-06-08T20:02:58.000Z",
"mergeRequestUser": "akudev",
"hasMergeConflict": false,
"isFirstContribution": true,
"tooManyFiles": false,
"popularityLevel": "Well-liked by everyone",
"pkgInfo": [
{
"name": "openui5",
"kind": "edit",
"files": [
{
"path": "types/openui5/README.md",
"kind": "markdown"
},
{
"path": "types/openui5/index.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/jquery.sap.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/openui5-tests.ts",
"kind": "test"
},
{
"path": "types/openui5/sap.f.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.m.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.tnt.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.codeeditor.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.commons.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.core.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.dt.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.fl.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.integration.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.layout.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.mdc.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.rta.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.suite.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.support.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.table.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.testrecorder.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.unified.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.ui.ux3.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/sap.uxap.d.ts",
"kind": "definition"
},
{
"path": "types/openui5/tsconfig.json",
"kind": "package-meta-ok"
},
{
"path": "types/openui5/tslint.json",
"kind": "package-meta",
"suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-linter-tslintjson) and not moving towards it (check: `rules`)"
}
],
"owners": [],
"addedOwners": [
"openui5bot",
"petermuessig",
"codeworrior",
"akudev"
],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "amcasey",
"date": "2021-06-08T19:32:28.000Z",
"isMaintainer": true
}
],
"mainBotCommentID": 856700639,
"ciResult": "pass"
} |
|
🔔 @akudev — you're the only owner, but it would still be good if you find someone to review this PR in the next few days, otherwise a maintainer will look at it. (And if you do find someone, maybe even recruit them to be a second owner to make future changes easier...) |
|
@akudev The CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review. |
...because these type definition files cover an entire npm org and do not match exactly one package
|
I'm a bit puzzled why the "No Other Owners" label and the bot say that I am the only owner when the bot also noted that four owners are added. |
|
@akudev The source of truth for ownership is actually https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/.github/CODEOWNERS, which won't be updated until after this PR merges. |
amcasey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed explanation! Please keep working on those lint suppressions.
|
Ready to merge |
|
@amcasey: thanks for the quick approval! |
|
I just published |
Please fill in this template.
npm test <package to test>.If changing an existing definition:
This PR brings an update for @openui5/types, which is still on a very old outdated version 1.40.
The previous definition files were done by community member @maylukas (thanks!!) who is also the sole owner of the definitions right now, but has stopped working on this project years ago. We contacted him and he confirmed that we could/should take over ownership, hence he is now being removed from the list.
We (@codeworrior, @petermuessig and me) are members of the original OpenUI5 development team, but due to the structure and ways of consumption of this JS library it is not feasible to bundle the declaration files inside, so we would like to continue to use DefinitelyTyped instead.
The definition files need to cover a 10+-years-grown API and are generated, hence not all guidelines can easily be fulfilled and not all common mistakes avoided. We are working on it and on reducing the number of disabled linting rules.
One example is the "no-any-union" rule: while it completely makes sense from pure type-checking perspective, most violations are for cases when a value of type "any" can be either set or bound. For setting, "any" is needed, but for binding, it's still good to have the type "PropertyBindingInfo" present for documentation purposes.
Another example: the "ban-types" rule complains about type "Function": while we could enable and satisfy the rule by saying "(..any) => any|void" instead, this does not actually improve quality/type-safety. Instead, we plan to work on providing the full function signatures in those places (we have to solve an issue in JSDoc for that).
The tests from the previous definition files have been taken over and adapted as closely to the original as possible.
Two links to a "helloworld" project in the README are not working right now, but this project is already finished and about to be published in the next 2-3 days.