-
Notifications
You must be signed in to change notification settings - Fork 94
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
ReadMes overview page update #1199
Conversation
I think we need the following info in the landing page:
Sample: https://github.com/kriasoft/nodejs-api-starter Or maybe something simple like the COBOL course: https://github.com/openmainframeproject/cobol-programming-course |
ok, so looks like we need not just the repo ReadMes overview but the entire repo overview that includes the bullets (maybe not all) suggested by @jellypuno Is that a correct understanding? |
@jellypuno There are TODOs in the "readme update..." commit, that have to with the License section and How to Contribute section. I'd like to get your feedback on the idea suggestion in the Contribute section. |
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.
Heyoo @IgorCATech, a few comments...sorry if I'm being too nitpicky ✌️ 🌮
README.md
Outdated
|
||
- **Delete a profile**: Right-click a chosen profile and select **Delete Profile** to permanently delete the profile. The functionality deletes a profile from your `.zowe` folder. | ||
{TODO Do we want to create some sort of labels that will indicate that 'external' help is needed/wanted? Like "help needed" or something like that.} |
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.
Maybe we can include a list of the labels that we use to mark issues people can contribute to?
Like help wanted
I think is for this, or good first issue
Should probably confirm with other people on the squad to decide which labels are appropriate...
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.
I agree! So, I've added some information about those labels. Now, the question here is whether someone will keep an eye on those labels. So far none of the 2 labels has been added to any issue.
@jellypuno Let me know what you think about including information about these labels in the doc. If that's a good idea, I suggest then asking the squad members to check the current open issues and to add the abovementioned labels if applicable.
@jellypuno It's still not clear what text to include in the License section (Lines 83-89). Any suggestions as to who can know this? I guess, the worst-case scenario - we remove this section altogether. |
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.
I think this updated readme has a lot of good information in it! I made a couple suggestions to consider, but overall, I think it's a big improvement.
Thanks @IgorCATech for these updates!
README.md
Outdated
|
||
![Save](/docs/images/ZE-safe-save.gif?raw=true "Save") | ||
<br /><br /> | ||
## Build Locally |
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.
Should this "Build Locally" section be moved under the Zowe Explorer package's readme? The instructions are specific to building that vsix file.
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.
I think the rationale behind having this section in this readme is to provide developers with an excerpt from the Dev's readme and include the link to the Dev's readme at the same time.
Actually, originally this section was part of the core ZE readme (package's readme now) but we decided to exclude it from there in order not to overload users with more options to install the extension.
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.
Personally, I think it makes more sense to put this under the Zowe Explorer package's readme. However, if the squad prefers to keep it in the main repo readme, then perhaps we could add a line specifying that these instructions are just for building the Zowe Explorer VS Code extension .vsix
file.
README.md
Outdated
|
||
### Create a new PDS and a PDS member | ||
Ensure that you have met the [software requirements](#requirements) before you can build and test your Zowe Explorer. |
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.
Maybe remove "can" from this line.
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.
Good catch!
README.md
Outdated
|
||
[Zowe Explorer](https://github.com/zowe/community#zowe-explorer) is a sub-project of Zowe, focusing on modernizing mainframe experience. [Zowe](https://www.zowe.org/) is a project hosted by the [Open Mainframe Project](https://www.openmainframeproject.org/), a [Linux Foundation](https://www.linuxfoundation.org/) project. | ||
Welcome to Zowe Explorer! This project is powered by Zowe, [Open Mainframe Project](https://www.openmainframeproject.org/). |
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.
I prefer the wording: "Zowe, a project hosted by the Open Mainframe Project".
The format "Zowe, Open Mainframe Project" reminds me of the format people use when listing their address (e.g. City, State or City, Country).
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.
I think the emphasis here should be put on Zowe Explorer rather than Zowe :) Maybe, something like "Welcome to Zowe Explorer, a project hosted by Open Mainframe Project!"?
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.
Sure, that works for me.
packages/zowe-explorer/README.md
Outdated
@@ -66,6 +66,17 @@ Create a profile, review the sample use cases to familiarize yourself with the c | |||
|
|||
You can now use all the functionalities of the extension. | |||
|
|||
#### Profile Validation | |||
|
|||
Zowe Explorer includes the profile validation feature that helps to ensure that z/OSMF is accessible and ready for use. If a profile is valid, the profile is active and can be used. By default, the feature is automatically enabled. You can disable the feature by right-clicking on your profile and selecting the **Disable Validation for Profile** option. Alternatively, you can enable or disable the feature for all profiles in the VS Code settings. |
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.
We should use a more general term instead of "z/OSMF" here. Perhaps:
"helps to ensure that the specified connection to z/OS is accessible and ready for use."
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.
Then I would suggest "helps to ensure that the specified connection to z/OS is successfully established and ready for use."
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.
Sounds good to me!
README.md
Outdated
2. Open the **DATA SETS** bar. | ||
3. Click the **Add Profile** button on the right of the **DATA SET** explorer bar. | ||
4. Select the profile that you want to add to the view as illustrated by the following screen. | ||
## Available Documentation |
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.
Since we say we are linking to documentation here, it might be helpful to link directly to the readme part (i.e. anchor tag) of these repo package pages. For example:
- For the Zowe Explorer readme: https://github.com/zowe/vscode-extension-for-zowe/tree/master/packages/zowe-explorer#zowe-explorer
- For the extensibility API readme: https://github.com/zowe/vscode-extension-for-zowe/tree/master/packages/zowe-explorer-api#extensibility-api-for-zowe-explorer
And so on.
README.md
Outdated
Your PDS member (or PS) is uploaded. | ||
|
||
**Note:** If someone else has made changes to the PDS member (or PS) while you were editing it, you can merge your conflicts before uploading to the mainframe. | ||
- Install [Yarn](https://yarnpkg.com/getting-started/install). |
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.
This section might be clearer if we group the client-side prerequisites together separately from the host prerequisites instead of mixing the two together. Node.js and Yarn will be needed locally on the client-side. The (current) middle bullet point about TSO/E, etc. is host-side.
README.md
Outdated
|
||
- **Associate profiles**: You can create a secondary association by right-clicking the profile and selecting the **Associate profiles** icon. For more information, see [the Associate profiles section](https://docs.zowe.org/stable/user-guide/ze-profiles.html#associate-profile) in Zowe Docs. | ||
{TODO I don't know exactly how to formulate this section exactly. See the following 2 versions. Something tells me, it's the second version but I'd rather check with someone who knows for sure} |
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.
Unfortunately, I'm also not sure what to put in this section. Zowe CLI's repo doesn't appear to have a license section in its readme. @jellypuno Perhaps if we are all unsure, we can reach out to Zowe leadership for guidance on this?
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.
I was told to scrap this section.
For the Zowe Explorer package readme, I feel we should make the link to Zowe Docs' Zowe Explorer pages more obvious. Maybe we can put a "What's next?" section under the "Usage Tips" section, and say something like:
|
@lauren-li We have a note under the Usage section with a link to the configuration page (part of the install doc on the ZE-related Zowe Docs page), I think I should replace it with your suggestion to make users aware that there is more to Zowe Explorer than what's described in this readme 👍 Great idea! |
I understand that there's a problem with running the |
Added a draft of the release note for 1.13. Can I ask @jellypuno and/or @lauren-li to review, please? Also, I lack information on the 'zftp extension for data set'. Is that a new feature in ZE or is that an extension for ZE, that introduces new features? Many thanks! |
@IgorCATech This changelog item is actually for the Zowe Explorer FTP extension package. I don't think it fits on Zowe Explorer's Readme page. It allows the Zowe Explorer FTP extenion user to use their zFTP profile in Zowe Explorer's Data Sets view. |
README.md
Outdated
3. Open the profile and PDS containing the member. | ||
4. Right-click on the PDS member that you want to delete and select **Delete Member**. | ||
5. Confirm the deletion by clicking **Yes** on the drop-down menu. | ||
Clone the repository and run `yarn workspace vscode-extension-for-zowe package` to build a VSIX file and start working with the extension. |
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.
I tried this command and I ended up with just one .vsix file. It is better to replace yarn workspace vscode-extension-for-zowe package
to yarn run package
. And after that, maybe it is good to mention that the vsix is in the dist
folder
packages/zowe-explorer/README.md
Outdated
- Fixed the issue that caused removed favorite profiles to persist between IDE sessions. | ||
- Fixed the issue that prevented updated credential prompting from occurring when a profile was marked “invalid”. | ||
- Added the monorepo landing Readme that contains the high-level overview of the repository folders such as `packages` folder, instructions on how to contribute to the project and links to Medium articles providing additional useful information about Zowe Explorer and Zowe. | ||
- Added the previously selected `Reject Unauthorized` value to the placeholder text of the entry field while updating an existing profile. |
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.
This is actually a bug fix. It loads the current RejectUnauthorized value from your profile and also bring it to the top of the selection and highlight it.
99629b0
to
a243357
Compare
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.
Read through it again and it seems okay to me :) Can give a thumbs up 👍
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.
Looks good to me!
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.
Top level readme looks good. I just had some minor comments on the changelog and release notes.
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 addressing my comments @IgorCATech . LGTM
README.md
Outdated
|
||
## Usage tips | ||
[Zowe Explorer FTP Extension ReadMe](https://github.com/zowe/vscode-extension-for-zowe/blob/master/packages/zowe-explorer-ftp-extension/README.md) — contains information about how to install and use the Zowe Explorer extension for FTP. The extension adds the FTP protocol to Zowe Explorer, enabling you to use z/OS FTP Plug-in for Zowe CLI profiles to connect and interact with z/OS USS. |
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.
@IgorCATech Note: If PR #1219 is merged in, this line should be updated to say "interact with z/OS USS and MVS". (This update can be in a separate PR, though.)
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 heads-up, @lauren-li I think if we are sure that #1219 will be merged together with the ZE 1.13 release PR, I can update this line with #1235 If we are not so sure, let me update this information in another PR. How does that sound?
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.
@IgorCATech That would be perfect, thank you!
README.md
Outdated
For more information, see [Changelog](https://marketplace.visualstudio.com/items/Zowe.vscode-extension-for-zowe/changelog). | ||
|
||
## Prerequisites | ||
Client-side prerequisites: |
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.
Should we add VS Code here, as well? Seems like a given, but at the same time, you really can't use Zowe Explorer without it. (Although, technically there are alternative options like VSCodium and Theia, too.)
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.
I wouldn't add 'VS Code' as a prerequisite because this ReadMe is part of Marketplace. Seems redundant to me. Otherwise, I would list VS Code in the pre-req section
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.
@IgorCATech Some users may come to this page on GitHub outside of the VS Code context, so I think having it listed could be helpful in those cases. But you have a point, as well.
README.md
Outdated
@@ -1,296 +1,94 @@ | |||
# Zowe Explorer | |||
# Getting Started with Zowe Explorer |
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.
Simply "Zowe Explorer" seems like a better top-level header to me. Most other Zowe repos I've looked at (e.g. Zowe CLI, Zowe docs) just say the name of the project here. "Getting started" might be more fitting as a subheader.
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.
Maybe even "Zowe Explorer Overview"? In my understanding, this readme's goal is to provide users with a set of tools and a map of other readmes and useful Zowe Explorer-related links.
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.
@IgorCATech That works for me. Thank you for explaining your thinking behind this!
README.md
Outdated
|
||
**Note:** Alternatively, you can select 'No' to cancel the deletion. | ||
```shell | ||
git clone --origin=upstream --branch=main --single-branch https://github.com/zowe/vscode-extension-for-zowe.git |
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.
@IgorCATech We don't currently have a branch named main
, so running this command will result in an error: warning: Could not find remote branch main to clone.
. This command should probably be:
git clone --origin=upstream --branch=master --single-branch https://github.com/zowe/vscode-extension-for-zowe.git
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.
ooh.. they've renamed it back to 'master'. Good catch.
README.md
Outdated
3. Open the profile and PDS containing the member. | ||
4. Right-click on the PDS member that you want to delete and select **Delete Member**. | ||
5. Confirm the deletion by clicking **Yes** on the drop-down menu. | ||
1. Clone the repository by issuing the following command: |
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.
by issuing the following command:
Suggestion: "by issuing the following command in your local command line interface:"
(Just to be clear for those who are less familiar with this process.)
README.md
Outdated
|
||
6. To delete a PDS, right-click the PDS and click **Delete PDS**, then confirm the deletion. | ||
2. From your local copy of the repository, issue the following commands: |
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.
I would add a step above this to change directories into the newly-cloned repository. (Again, just to make it clear for those who are not familiar with the process.) Something like:
"Change directories into the newly-cloned repository: cd vscode-extension-for-zowe
"
packages/zowe-explorer/CHANGELOG.md
Outdated
- Added the monorepo landing Readme that contains the high-level overview of the repository folders such as `packages` folder, instructions on how to contribute to the project and links to Medium articles providing additional useful information about Zowe Explorer and Zowe [#1199](https://github.com/zowe/vscode-extension-for-zowe/pull/1199). Thanks @IgorCATech | ||
- Fixed the issue that prevented the list of recently opened files from being displayed upon request. You can access a list of recently opened files by pressing the Ctrl+Alt+R or Command+Alt+R key combination [#1208](https://github.com/zowe/vscode-extension-for-zowe/pull/#1208). Thanks @jellypuno | ||
- Fixed the issue that prevented file picker from functioning. The file picker feature lets you filter your datasets in the tree by pressing the Ctrl+Alt+P combination [#992](https://github.com/zowe/vscode-extension-for-zowe/issues/992). Thanks @katelynienaber | ||
- Fixed the issue that caused the content from a previously filtered directory instead of the currently filtered directory to be served [#1134](https://github.com/zowe/vscode-extension-for-zowe/issues/1134). Thanks @lauren-li |
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.
filtered directory
Change to: "filtered USS directory"
packages/zowe-explorer/README.md
Outdated
- Fixed the issue that prevented the list of recently opened files from being displayed upon request. You can access a list of recently opened files by pressing the Ctrl+Alt+R or Command+Alt+R key combination. | ||
- Fixed the issue that prevented file picker from functioning. The file picker feature lets you filter your datasets in the tree by pressing the Ctrl+Alt+P combination. | ||
- Fixed the issue that caused the content from a previously filtered directory instead of the currently filtered directory to be served. |
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.
See my comments for the Zowe Explorer 1.13.0 changelog.
packages/zowe-explorer/README.md
Outdated
|
||
For more information about SCS, see [Secure Credential Store Plug-in for Zowe Explorer](https://docs.zowe.org/stable/user-guide/ze-profiles.html#enabling-secure-credential-store-with-zowe-explorer). | ||
|
||
## Usage tips | ||
|
||
- Use the **Add to Favorite** feature to permanently store chosen data sets, USS files, and jobs in the **Favorites** folder. Right-click on a data set, USS file or jobs and select **Add Favorite**. | ||
|
||
- **Syntax Highlighting:** Zowe Explorer supports syntax highlighting for data sets. Fox example, you can use such extensions as [COBOL Language Support](https://marketplace.visualstudio.com/items?itemName=broadcomMFD.cobol-language-support) or [HLASM Language Support](https://marketplace.visualstudio.com/items?itemName=broadcomMFD.hlasm-language-support). | ||
- **Syntax Highlighting:** Zowe Explorer supports syntax highlighting for data sets. You can search and install such extensions in VS Code Marketplace. |
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.
You can search and install
Change to: "You can search for and install"
packages/zowe-explorer/README.md
Outdated
@@ -339,7 +355,9 @@ For more information about SCS, see [Secure Credential Store Plug-in for Zowe Ex | |||
|
|||
- **Associate profiles**: You can create a secondary association by right-clicking the profile and selecting the **Associate profiles** icon. For more information, see [the Associate profiles section](https://docs.zowe.org/stable/user-guide/ze-profiles.html#associate-profile) in Zowe Docs. | |||
|
|||
For information on how to configure Zowe Explorer, see [Zowe Explorer Configuration](https://docs.zowe.org/stable/user-guide/ze-install.html#configuration). | |||
- **Open recent members**: Zowe Explorer lets you open a list of members you worked on earlier. You can access the list by pressing Ctrl+Alt+R or Command+Alt+R. |
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.
Ctrl+Alt+R or Command+Alt+R
Change to: "Ctrl+Alt+R (Windows) or Command+Option+R (Mac)"
packages/zowe-explorer/README.md
Outdated
For information on how to configure Zowe Explorer, see [Zowe Explorer Configuration](https://docs.zowe.org/stable/user-guide/ze-install.html#configuration). | ||
- **Open recent members**: Zowe Explorer lets you open a list of members you worked on earlier. You can access the list by pressing Ctrl+Alt+R or Command+Alt+R. | ||
|
||
For the complete Zowe Explorer documentation that also includes information about USS and Jobs interactions, see [the complete Zowe Explorer documentation](https://docs.zowe.org/stable/user-guide/ze-install.html). |
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.
To avoid saying "the complete Zowe Explorer documentation" twice, we could phrase this as:
"For additional Zowe Explorer documentation that also includes information about USS and Jobs interactions, see the complete Zowe Explorer documentation."
@IgorCATech I couldn't figure out how to retroactively add an overall comment with the review I just did, but in summary, I think this PR is shaping up well! I commented on some improvements I think we can make for the accuracy and clarity of our |
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
@IgorCATech I know my comments look like a lot, so if it would help you for me to make the edits straight into this PR or create a PR into yours, I would be happy to. Just let me know! I tried to provide suggestions with all of my comments, so in most (if not all) cases, it should just be a matter of copy/pasting the suggestions you agree with. |
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.
This looks good to me now! Thank you @IgorCATech for these improvements and updates, and for being so responsive in working with me to address all the comments!
ReadMes overview page update
ReadMes overview page update Signed-off-by: zFernand0 <fernando.rijocedeno@broadcom.com>
Signed-off-by: Igor Kazmyr 43951184+IgorCATech@users.noreply.github.com
Proposed changes
This PR aims to fix the landing page Readme. Currently, the Readme reflects the contents of the Zowe Explorer core readme (one of the packages). The idea is to have a high-level overview ReadMe that includes information about existing ReadMes in the repository.
Release Notes
Milestone:
1.13.x
Changelog:
Types of changes
What types of changes does your code introduce to Zowe Explorer?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the revieweryarn workspace vscode-extension-for-zowe vscode:prepublish
has been executedFurther comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...