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

fix: broken links in docs #99

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

skodamarthi
Copy link
Contributor

@skodamarthi skodamarthi commented Oct 27, 2022

No description provided.

@skodamarthi skodamarthi mentioned this pull request Oct 27, 2022
@skodamarthi
Copy link
Contributor Author

@adierkens Can you please review?

@KetanReddy KetanReddy linked an issue Oct 28, 2022 that may be closed by this pull request
@skodamarthi
Copy link
Contributor Author

@KetanReddy @hborawski Tried adding the new file with required documentation. Can you please check if it looks ok?

@skodamarthi
Copy link
Contributor Author

@hborawski Thanks for adding the hacktoberfest label. For the contribution to be counted, "hacktoberfest-accepted" label is needed instead. Can you please add that if the changes look ok to be merged? Thanks again!

@KetanReddy
Copy link
Member

KetanReddy commented Oct 28, 2022

@KetanReddy @hborawski Tried adding the new file with required documentation. Can you please check if it looks ok?

You're going to want to remove the references to the non-opensourced components that are referenced. Also it looks like there is a header without any text underneath it? This is something we should probably formalize in our docs, but ideally doc pages should follow the language conventions that the rest of the docs use. Things like:

  • Not using "things" as its too generic
  • Refer to the project as "Player" (as a proper noun) not as a reference "the player"

@skodamarthi
Copy link
Contributor Author

skodamarthi commented Oct 28, 2022

@KetanReddy @hborawski Tried adding the new file with required documentation. Can you please check if it looks ok?

You're going to want to remove the references to the non-opensourced components that are referenced. Also it looks like there is a header without any text underneath it? This is something we should probably formalize in our docs, but ideally doc pages should follow the language conventions that the rest of the docs use. Things like:

  • Not using "things" as its too generic
  • Refer to the project as "Player" (as a proper noun) not as a reference "the player"

@KetanReddy I didn't write this content but took it from the link given in the issue: https://ibp-cicd-core.prod1-ibp.a.intuit.com/player/player/latest/index.html#/core/architecture/cross-platform-players as it is accessible now. I am not too sure what the non-opensourced components are, can you help me understand what exact changes need to be made? Shall I remove all the content that has Intuit github links? I can change "the player" to "Player", but unclear about other things as I copy pasted it from the original source and also used the same header format as in other doc files in this repo.

@skodamarthi
Copy link
Contributor Author

@KetanReddy Tried changing the content as per your suggestions. Please let me know if any additional changes are required as I am not too familiar with the functionality of the plugin and required content for this file.


## What this looks like in practice

Plugins such as the common-types plugin is a great example of a small plugin used to add a very specific feature to the core-player. Since the validations, formats, and data-types present in this plugin are authored once, we can leverage Player's architecture to share these implementations across platforms.
Copy link
Member

Choose a reason for hiding this comment

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

Should be a link to the common types plugin here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

skodamarthi and others added 4 commits October 31, 2022 21:19
Co-authored-by: Ketan Reddy <KetanReddy@users.noreply.github.com>
Co-authored-by: Ketan Reddy <KetanReddy@users.noreply.github.com>
Co-authored-by: Ketan Reddy <KetanReddy@users.noreply.github.com>
@skodamarthi
Copy link
Contributor Author

@KetanReddy Done the suggested changes. Can you please check?

@skodamarthi
Copy link
Contributor Author

@KetanReddy @hborawski Can you please review and merge if this looks ok?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix links in docs
3 participants