-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[CLA Approved] Add openmct type hints #7247
Conversation
Current Playwright Test Results Summary✅ 162 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 12/01/2023 09:14:00pm UTC)
|
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Timer Can perform actions on the Timer
Retry 1 • Initial Attempt |
0% (0)0 / 37 runsfailed over last 7 days |
18.92% (7)7 / 37 runsflaked over last 7 days |
📄 functional/plugins/notebook/restrictedNotebook.e2e.spec.js • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Restricted Notebook with a page locked and with an embed @addinit Disallows embeds to be deleted if page locked @addinit
Retry 1 • Initial Attempt |
0% (0)0 / 39 runsfailed over last 7 days |
28.21% (11)11 / 39 runsflaked over last 7 days |
📄 functional/plugins/notebook/notebookSnapshots.e2e.spec.js • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Snapshot image tests Can drop an image onto a notebook and create a new entry
Retry 1 • Initial Attempt |
2.50% (1)1 / 40 runfailed over last 7 days |
62.50% (25)25 / 40 runsflaked over last 7 days |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7247 +/- ##
==========================================
- Coverage 55.40% 55.38% -0.02%
==========================================
Files 671 671
Lines 27010 27010
Branches 2631 2631
==========================================
- Hits 14964 14959 -5
- Misses 11324 11327 +3
- Partials 722 724 +2
*This pull request uses carry forward flags. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Thank you so much for your contribution! If you haven't already, please complete and submit a Contributor License Agreement (CLA). This is required before we can approve/merge your code. That will take a little bit of time to process, so in the meantime if you'd like to add any more types in this PR please feel free to do so. Thanks again! |
Hello @ozyx , Thank you for reviewing the PR. We're working with Open MCT and TypeScript at our rocketry club, so I will try to add type information for APIs as I go. I emailed the Contributor License Agreement to arc-sra-team@mail.nasa.gov and kimberly.minafra@nasa.gov, hope they can review it soon. |
Also added type for the TypeRegistry |
@ozyx Any updates on the CLA? |
@skrobchik No updates yet, I just sent a follow-up so I'll let you know once they get back to me on the CLA status. We have a few folks waiting on approval. Unfortunately this is largely a manual process and can take some time, especially during the holidays. Appreciate your patience! |
I've followed up on the email to arc-sra-team@mail.nasa.gov and kimberly.minafra@nasa.gov, but still haven't received a response. |
I will also follow up and see if I can get a confirmation. Apologies again for the wait. |
@skrobchik Your CLA has been approved! Thank you for waiting. |
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.
LGTM
@skrobchik Please merge master into this branch or rebase it on top of master-- otherwise, this is good to go! |
Rebased |
@skrobchik I don't have write access to the branch you're trying to merge from, so this is proving difficult as master continually changes. Please do one of the following:
|
@ozyx I'm not quite sure what you mean with option 2 ? I rebased on top of master anyway. I thought that GitHub automatically rebased pull-requests on top of master before merging if there are no merge conflicts. |
Yeah you're doing everything correctly, what I mean is that since the repo it's merging from is protected, if master were to change while I'm waiting for our CI checks to complete I wouldn't be able to rebase or add a merge commit because I don't have write access to your fork. Regardless-- option 1 worked :) |
Closes
Describe your changes:
Adds type information for the
setAssetPath
method.All Submissions:
Author Checklist
Reviewer Checklist