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

Typo Fixes #1012

Merged
merged 2 commits into from
Jun 21, 2023
Merged

Typo Fixes #1012

merged 2 commits into from
Jun 21, 2023

Conversation

ashok-hariharan
Copy link
Contributor

@ashok-hariharan ashok-hariharan commented Jun 19, 2023

Fixing two issues as part of my first contribution to Open Source

  1. Added unit test policy Unit Test Policy #832 (will raise a separate PR for this issue as it needs more details)
  2. Typo in multiple files Typo in docs/context /spec.md #989

Please check this @robmoffat

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 19, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@netlify
Copy link

netlify bot commented Jun 19, 2023

Deploy Preview for fdc3 canceled.

Name Link
🔨 Latest commit d91de10
🔍 Latest deploy log https://app.netlify.com/sites/fdc3/deploys/649041075e877a0008825979

Copy link
Contributor

@kriswest kriswest left a comment

Choose a reason for hiding this comment

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

Type fixes all look good.

We need a bit more thought on/detail in the unit test policy before merging that. Hence, you might consider splitting this into two PRs so that they can be handled separately.

CONTRIBUTING.md Outdated
**4.3. Reviewing/Discussion.**
**4.3. Unit Test Policy**

- Any change to the FDC3 specification typescript codebase must be supoorted by accompanying unit tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple of types to fix:

Suggested change
- Any change to the FDC3 specification typescript codebase must be supoorted by accompanying unit tests.
- Any change to the FDC3 specification's typescript codebase must be supported by accompanying unit tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed unit test policy changes. Will keep it as a separate PR by adding some more additional details to it

CONTRIBUTING.md Outdated
**4.3. Reviewing/Discussion.**
**4.3. Unit Test Policy**

- Any change to the FDC3 specification typescript codebase must be supoorted by accompanying unit tests.
Copy link
Contributor

@kriswest kriswest Jun 19, 2023

Choose a reason for hiding this comment

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

Some more detail would also be useful here - FDC3 is made up of 4 specifications, one of which relates to the Desktop Agent API. Most of the codebase is just typing to define that API standard. However, there is actual code in method.ts, which is currently all that we apply unit tests to. There is also code in ContextTypes.ts, which is auto-generated from context schemas. This does not currently include tests - and we might not want to be forced to add them when new Context type schemas are added and start auto-generating new functions.

There are other apps in the /toolbox directory. These don't currently have unit tests.
We might add further apps there, e.g. a basic app directory implementation - the existing out-of-date implementation was recently removed.

Copy link
Contributor

@kriswest kriswest left a comment

Choose a reason for hiding this comment

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

LGTM! Will update title to reflect removal of Unit test policy

@kriswest kriswest changed the title Unit Test and Typo Fix Typo Fixes Jun 20, 2023
@kriswest kriswest requested review from hughtroeger and a team June 20, 2023 11:02
@kriswest kriswest linked an issue Jun 20, 2023 that may be closed by this pull request
@kriswest kriswest added the docs Documentation label Jun 20, 2023
@ashok-hariharan
Copy link
Contributor Author

I don't have write access, requesting @greyseer256 or @kriswest to merge it

@kriswest kriswest merged commit dffe686 into finos:master Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typo in docs/context /spec.md
3 participants