magento/devdocs#5245: Hard tabs. Folder: guides/cloud #5471
Conversation
magento-engcom-team
added
Partner: Atwix
partners-contribution
|
An admin must run tests on this PR before it can be merged. |
There was a problem hiding this comment.
Care must be taken about code blocks and images and other paragraphs inside of lists. This breaks many ordered lists because the code blocks and the images are flush left. It is not enough to remove hard tabs, you may have to add spacing back in so that these types of elements align properly.
There was a problem hiding this comment.
@atwixfirster Like @hguthrie mentioned. I flagged one file where it's easy to see the misalignment. We can review again after you fix. Thx.
|
The following screenshot shows the result of removing hard spaces from an ordered list with images in a step:
As you can see, the alignment is off and the list numbering starts over. |
@hguthrie , could you please verify if I will implement the same styles to the rest pages. Thank you! |
@atwixfirster Yes! The topic you fixed looks like it should; the ordered lists are ordered. That is a great help to us. 😄 You're on the right track. If you run into anything weird in fixing the remaining topics, feel free to ask us about it. |
atwixfirster
force-pushed
the
hard-tabs-guides-cloud
branch
from
cfc3c7f to
6a9dbaa
Compare
|
@hguthrie, PR is ready for your review. Many thanks! |
|
@atwixfirster Okay. At the moment, your branch fails the markdown linter test. |
Could you please attach a report with issues? Thanks! |
|
@atwixfirster It is best if you run the preview and tests in your local environment. Follow the steps defined in the GitHub issue 5245. |
hguthrie
added
the
Editorial
atwixfirster
force-pushed
the
hard-tabs-guides-cloud
branch
from
7130c03 to
2901af6
Compare
@hguthrie , could you please run tests again? Thank you! |
|
@atwixfirster I am seeing artifacts from previous merges to master. And I am seeing changes that have nothing to do with hard tabs. This could be a result of rebasing, but, whatever the issue, we cannot merge this PR and risk the impact to the Cloud guide. |
@hguthrie , I hope you are joking... You requested to fix content after remove hard tabs. And I did that. Now you tell me what you can not merge this PR. !!! Please tell me that the last your message was a joke. Thank you! |
|
Hey Alex, I am not joking, in fact! There are changes in this PR that are not related to the hard tabs issue. Because of the nature of this infrastructure work, we can only accept changes in this PR related to hard tabs. I know you have worked hard to revisit the changes in Cloud guide, and the changes to ordered lists appear to be fine. The earlier tests were failing because of spacing inserted into blank lines before and after code blocks. However, I reviewed it again and I am still not able to approve this PR for merge. For example, some YAML samples were reformatted with the incorrect spacing, some unnecessary spaces were added to em dashes in lists. I see changes to lexers for some Code blocks, too. These are changes that are not related to the Hard Tab rule issue. If you’d like to remove changes not applicable to the related issue, I can re-review for merging. Or, if you’d rather start fresh, we can do that too. Please close this and submit another PR. |
|
Ordinarily, I would agree with @hguthrie that we should limit the scope of PRs. However, these markdown linting issues are unique. We (the Magento Docs team) didn't provide sufficient info in the issue to warn contributors about scope, so I'm going to let that slide for this PR. @atwixfirster, if you will resolve the merge conflicts, I'll re-review your PR. This should resolve all the remaining MD010 errors that are left, which will allow us to enable the rule on production and start making progress on the next linting rule. |
Merge remote-tracking branch 'upstream/master' into hard-tabs-guides-cloud # Conflicts: # guides/v2.2/cloud/cdn/cloud-fastly-custom-response.md # guides/v2.2/cloud/cdn/configure-fastly.md # guides/v2.2/cloud/cdn/fastly-vcl-wordpress.md # guides/v2.2/cloud/requirements/cloud-requirements.md # guides/v2.3/cloud/requirements/cloud-requirements.md # guides/v2.3/cloud/setup/first-time-deploy.md
@jeff-matthews you are the man with a BIG heart. I put a lot of my time to this one. I've resolved conflicts. Could you please review? Thank you! |
There was a problem hiding this comment.
@atwixfirster, I reviewed your changes and I only have a few comments.
I also see unexpected commits from PRs that were merged to master yesterday. @atwixfirster, did you rebase your branch at some point?
@hguthrie & @meker12, please verify that the YAML spacing is correct in the guides/v2.2/cloud/configure/import-url-rewrites.md file.
@jeff-matthews , current branch was created from Did I do something wrong? Thank you! |
|
I'm not sure. If you rebased after someone else edited your branch, then maybe. We periodically sync the integration branch with master, so even if you created a working branch from a more recent version of master, those commits should go away after we sync the integration branch. |
|
Ok, those commits are now gone. I think this is a result of some idiosyncratic behavior in GitHub. This PR originally used |
|
Hi @atwixfirster, thank you for your contribution! |
|
Thanks @atwixfirster! |
Thank you @jeff-matthews , for your support here! |
Purpose of this pull request
This pull request (PR) is related to #5245: in the folder:
guides/cloud(v.2.2 and v.2.3)@meker12 , could you please review!
Affected DevDocs pages
Links to Magento source code
Thank you!