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

DOC Document making variants with different file extensions #441

Merged

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Jan 17, 2024

Description

This PR has two commits:

  1. Refactors file docs a little, so that there's a clear place for documentation about manipulating file content. I wasn't sure where to put the new docs so now there's a place that's clear for readers and maintainers alike.
  2. Documents NEW Allow file variants with different extensions silverstripe-assets#585

Please avoid recommending changes for content that I have simply lifted from one page and shifted to another.

Issues

Pull request checklist

  • The target branch is correct
  • All commits are relevant to the purpose of the PR (e.g. no TODO comments, unrelated rewording/restructuring, or arbitrary changes)
    • Small amounts of additional changes are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • The changes follow our writing style guide
  • Code examples follow our coding conventions
  • CI is green

@GuySartorelli GuySartorelli marked this pull request as draft January 17, 2024 21:56
@GuySartorelli GuySartorelli changed the title DOC Refactor docs about file manipulation into its own page DOC Document making variants with different file extensions Jan 17, 2024
@GuySartorelli GuySartorelli force-pushed the pulls/5/file-manipulation-docs branch 2 times, most recently from 6069172 to 8ec6d8a Compare January 18, 2024 02:08
@GuySartorelli GuySartorelli marked this pull request as ready for review January 18, 2024 02:10
@GuySartorelli GuySartorelli force-pushed the pulls/5/file-manipulation-docs branch from 8ec6d8a to bc37886 Compare January 18, 2024 02:11
@GuySartorelli
Copy link
Member Author

@michalkleiner Please stop suggesting rewording for content that I moved from one file to another. Changing that wording is not in scope.

Copy link
Contributor

@michalkleiner michalkleiner left a comment

Choose a reason for hiding this comment

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

Reads well. Mostly just minor stylistic suggestions and typo fixes.

@michalkleiner
Copy link
Contributor

michalkleiner commented Jan 18, 2024

@michalkleiner Please stop suggesting rewording for content that I moved from one file to another. Changing that wording is not in scope.

I reviewed the change as a whole. In my opinion if there are changes/tweaks that make sense to do to improve the overall quality of docs while we are touching the area, especially when they can be simply accepted in bulk via Github UI, I don't see that as a bad thing. Feel free to reject those if it bothers you too much.

@GuySartorelli
Copy link
Member Author

GuySartorelli commented Jan 18, 2024

I've made the changes that didn't require any thought, or which resolved potential confusion, or which were in scope.
I'm not making the rest of the suggested changes as they were out of scope and required some thought that I'd rather spend elsewhere.

@GuySartorelli GuySartorelli force-pushed the pulls/5/file-manipulation-docs branch from bc37886 to 7223e66 Compare January 18, 2024 04:32
@GuySartorelli GuySartorelli force-pushed the pulls/5/file-manipulation-docs branch from 7223e66 to 8d6b727 Compare January 25, 2024 01:55
@GuySartorelli GuySartorelli force-pushed the pulls/5/file-manipulation-docs branch from 8d6b727 to 0443770 Compare January 25, 2024 01:57
@emteknetnz
Copy link
Member

Will hold off merging in case anything comes up in peer review on silverstripe/silverstripe-assets#585 (review)

@emteknetnz emteknetnz merged commit 3e68745 into silverstripe:5 Jan 28, 2024
3 checks passed
@emteknetnz emteknetnz deleted the pulls/5/file-manipulation-docs branch January 28, 2024 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants