Skip to content

WIP: made migrating docs adhere to style guide #39

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

Closed
wants to merge 13 commits into from
Closed

WIP: made migrating docs adhere to style guide #39

wants to merge 13 commits into from

Conversation

jc-berger
Copy link
Contributor

@jc-berger jc-berger commented Jan 28, 2021

Fixes #182: Review Migration Guide. I've gone through the migration docs and made them more strictly adhere to the IBM style guide.

Note, I removed a lot of the "procedures" because there weren't actually instructions for user to do anything. We most certainly can include more instructions on how to migrate, but I'm going to need help with developing that content.

We also link a lot to GitHub issues. I'm fine with this, but with all links and GH issues especially, I'm concerned the issues we link to will quickly become outdated and therefore no longer useful.

@jc-berger jc-berger added the documentation Improvements or additions to documentation label Jan 28, 2021
@jc-berger jc-berger self-assigned this Jan 28, 2021
@jc-berger
Copy link
Contributor Author

I spoke with Robert Kratky. My follow up commit will focus exclusively on a language review. Any structural changes that's seen in this current commit can be addressed in a separate issue, down the line, after I've made these language changes.

Copy link
Contributor

@elsony elsony left a comment

Choose a reason for hiding this comment

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

@jc-berger A general comment: There is some work required in this migration guide to bring it to cover the entire devfile 2.x spec. The spec has changed since the last review of this documentation. I have talked to @l0rd before and we'll get some help from his team to update the content to this page since migration from devfile 1.0 to devfile 2.x is Che specific. This is a heads up that more changes may be coming in the migration doc.

@@ -1,12 +1,10 @@
[id="proc_migrating-plug-ins_{context}"]
= Migrating plug-ins
= Migrating plugins
Copy link
Contributor

Choose a reason for hiding this comment

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

Plugins have been removed from the specification (see devfile/api#333). The editor-specific configuration is taking a different approach. @l0rd This is one of the areas that need work on for the migration guide.

@jc-berger
Copy link
Contributor Author

New commit is up. No rush to review since PRs #43-46 take priority, given the needed schema updates.

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jc-berger

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment


.Procedure

* Replace `apiVersion: 1.0.0` by `schemaVersion: 2.0.0`:

. To migrate a schema version from devfiles v1.x to devfiles v2.x, replace `apiVersion: 1.0.0` with `schemaVersion: 2.0.0`.

Choose a reason for hiding this comment

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

where do I need to go to replace the schema version? Do you change a section in the devfile? This should be more clear I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I don't know the answer. It's probably something we can tackle in the separate issue for properly structure our procedures: devfile/api#402

@openshift-ci-robot
Copy link

@Eva-Lotte: changing LGTM is restricted to collaborators

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jc-berger
Copy link
Contributor Author

@Eva-Lotte, thanks for the awesome feedback! I've made a new commit, addressed doc review. I've resolved the conversations with your comments I addressed. At this point, I don't have answers to all your very good questions. I think some of the concerns can be shelved to a separate issue I opened for procedure restructure, since a lot of your concerns was the lack of adherence to procedure structure and purpose.

I left these comments unresolved so you can see what you think about holding off on these specific concerns.

Please let me know what you think after your second round of review, thanks!

@openshift-ci
Copy link

openshift-ci bot commented Apr 12, 2021

@jc-berger: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

This was referenced Apr 13, 2021
@jc-berger
Copy link
Contributor Author

Since there were too many difficult merge conflicts with this PR, I've opened #51. #51 addresses all the concerns brought up in this PR. Anyone please direct your attention there instead!

@jc-berger jc-berger closed this Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress documentation Improvements or additions to documentation needs-rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review Migration Guide for language and consistency
5 participants