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

New cut at documentation PR using current develop branch #198

Merged
merged 13 commits into from
Nov 30, 2018

Conversation

chazzmoney
Copy link
Collaborator

This is intended to replace #103. It is not intended to be a perfect or complete documentation update. The spirit is to get this in as quickly as possible and then add TODOs for any other documentation updates we want to do.

@didoo and @dbanksdesign Please give this a look over. It is based off of the existing setup in the develop branch, avoiding all the conflicts and fixing up some of the TODOs in the other branch.

I'd like to get this done and pulled before Wednesday next week.

I'll add issues for any comments I can't handle directly in this PR and tackle them small and separately from there.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@chazzmoney chazzmoney mentioned this pull request Nov 17, 2018
docs/README.md Outdated Show resolved Hide resolved
docs/architecture.md Outdated Show resolved Hide resolved
docs/assets/properties.md Outdated Show resolved Hide resolved
docs/assets/properties.md Outdated Show resolved Hide resolved
docs/assets/properties.md Outdated Show resolved Hide resolved
docs/config.md Outdated Show resolved Hide resolved
docs/config.md Show resolved Hide resolved
docs/package_structure.md Outdated Show resolved Hide resolved
docs/transform_groups.md Show resolved Hide resolved
docs/using_the_cli.md Outdated Show resolved Hide resolved
Copy link
Member

@dbanksdesign dbanksdesign left a comment

Choose a reason for hiding this comment

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

A few minor comments

bin/style-dictionary Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
docs/properties.md Show resolved Hide resolved
docs/properties.md Show resolved Hide resolved
docs/properties.md Outdated Show resolved Hide resolved
docs/properties.md Outdated Show resolved Hide resolved
docs/properties.md Outdated Show resolved Hide resolved
docs/quick_start.md Outdated Show resolved Hide resolved
@chazzmoney
Copy link
Collaborator Author

chazzmoney commented Nov 21, 2018

@dbanksdesign , found this at the end of the package_structure.md documentation:

Assets
Assets are not required, but can be useful to include in your style dictionary. If you don't want to manage having assets like images, vectors, font files, etc. in multiple locations, you can keep them in your style dictionary as a single source of truth.
Coming soon: how to generate image assets based on your style dictionary

Is there something to add here or should I leave as is?

@chazzmoney
Copy link
Collaborator Author

@didoo @dbanksdesign I think we are good here, unless you have any final comments?

@chazzmoney
Copy link
Collaborator Author

@didoo @dbanksdesign Am planning on squash and merging on Monday or Tuesday - let me know if you have any other thoughts or concerns.

## 3. Split the platforms

For each platform defined in your [config](config.md), Style Dictionary will do a few steps to get it ready to be consumed on that platform. Everything that happens in a platform is non-destructive so you don't need to worry about one platform affecting another.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dbanksdesign After reviewing the image with @didoo's comments, can you modify it to show that the iOS, android, and web are just examples?. I think maybe if you replaced them with purple bubbles of "Start Platform Build" and then put the value of "iOS" underneath maybe?

Also maybe the final step should be "built files can then be consumed in each platform"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we should add an initial step for reading or processing the configuration?

@didoo
Copy link
Contributor

didoo commented Nov 24, 2018

@chazzmoney I had a new look and seems ok to me. Only thing, I am still not 100% sure if this can generate confusion if we don't specify that it works only if we are targeting a "value":
https://github.com/amzn/style-dictionary/pull/198/files#diff-202d3626d667c87a265182819faab5e0R82

@chazzmoney
Copy link
Collaborator Author

Yeah, thats a bug. Tested and it creates a duplicate name. Let me see if I can find the issue and I'll submit a PR for it.

@chazzmoney
Copy link
Collaborator Author

Well... thats not going to happen. Because we replace references at the end, after everything is all done... there is no way to go back and reformat the names correctly (see #120). So we need to do references first / at the same time as transforms if we want this functionality to work.

@chazzmoney
Copy link
Collaborator Author

You can see my progress/the problem on that here:

develop...how-to-replace-non-value-references

@chazzmoney
Copy link
Collaborator Author

Updated the documentation to correctly reflect the current state of affairs. Should we create an issue for the alias thing? Or for the redo in @dbanksdesign's first comment in #120?

I'm leaning towards the latter; we had that conversation a year ago now and seems like it makes of sense to do for a 3.0.

@dbanksdesign
Copy link
Member

Let's leave the aliasing stuff as it is for now, we can think about how to solve it in 3.0. I think the main issue is what should happen if you try to reference a whole object that contains values? What are the problems we are trying to solve with aliasing a whole object with values in it?

* Final touches on build diagram and architecture

* Updating build diagram

* Updating build diagram
@chazzmoney chazzmoney merged commit 13479c4 into develop Nov 30, 2018
chazzmoney added a commit that referenced this pull request Dec 1, 2018
* improved error messages for registerTemplate

* updated error message

* Introduce option to control the generation of the "Do not edit" header (#132)

* stage #1 - formats.js

* stage #2 - templates

* reset changes to template + simplified changes to formats

(now the “options” object is assigned to the “file” element)

* fixed wrong parameter passed to fileHeader function

* updated documentation

* updates after PR comments

* removing the confusing static-style-guide stuff (#157)

* Fixes #72

* handle no command and invalid commands with friendly console output (#156)

* Add json5 support (#165)

* Removing unnecessary backticks (#172)

* Merge Jest Branch (#169)

* Jest testing (#133)

* moved all the existing tests to Jest
* finalised Jest tests for “utils” removing assert dependency
* finalised Jest tests for “register” removing assert dependency + moved tests under correct folder
* finalised Jest tests for “transform” removing assert dependency + moved tests under correct folder + removed extra file
* updated path for “service” files/folders
* removed output folder
* updated the paths to ignore in the Jest config in package.json
* finalised Jest tests for “clean” removing assert dependency + other small changes
* added “__output” to the list of folders ignored by Jest
* some tunings + more tests
* more tests cleanup
* fixed test for exportPlatform
* fixed last tests, and now all tests are green!
* Added first snapshot tests! Yay!
* added mock for dates to avoid failing snapshots tests
* updated tests
* first attempt to fix the UTC date problem on CI (reference: boblauer/MockDate#9)
* second attempt to fix the UTC date problem on CI
* removed the TZ=UTC env environment to test if is really needed
* updated all the occurrences of new Date in the templates
* restored linting before running the tests suite
* code style fix
* fixed wrong porting of the test for buildAllPlatforms

* test(all): Fix for all tests to match the date and remove of mockdate (#148)

inspiration jestjs/jest#2234

* test(javascript/es6): Add test for es6 (#149)

* test: add registerTemplate (#147)

* add tests for transform object (#151)

* add tests for transform object
* split up complex test in multiple smaller tests

* Jest flatten props (#163)

* Adding tests for lib/utils/flattenProperties.js (#146)

* Adding tests for lib/utils/flattenProperties.js

* update to use lodash sortby function

* update to use lodash sortby function

* Add babel-jest (#173)

* feat(json-nested): Add JSON nested transform (#167)

Added JSON nested transform, Added test for it and Documentation update

re #139

* Fix errors and improve error messaging (#158)

* updated error messaging. Fixes for issues with references.

* adding in didoo's test from #118

* cleanup of terminology

* fixed resolveObject to correctly replace multiple references. modified testing suite to reflect new test.

* updates per comments by didoo and dbanksdesign

* case sensitive, oops.

* case sensitive, oops.

* minor updates based on PR feedback

* merging with develop to ensure we stay synched

* removing cli error handling and moving to module

* removing per dannys comments

* making constants for group errors per Dannys comments

* switch to error grouping mindset and naming

* switch to error grouping mindset and naming

* per danny's comment

* fix flush to execute across all groups if called with no group; remove flush on uncaught exceptions to prevent confusion

* simplify, simplify, simplify

* changed out error naming to message mindset, cleaned out console.log, fixed issues with simplified GroupMessages

* sepearate circular reference tests into separate expects

* avoid using string so we dont get it confused with String

* Deprecating templates (#152)

* Displaying a warning when using templates in the config or registerTemplate
* Moving built-in templates to formats

* Porting over a stragler test (#190)

* 2.5.0

* Added 'json/flat' format (#192)

* Fix: #195 (#196)

* updating contributing to reflect the package manager and testing suite correctly (#197)

* Add Sass maps formats (#193)

* added ‘sass/map-flat’ and ‘sass/map-deep’ formats + updated tests

* fixed inconsistend newlines in templates for sass maps

* improved recursive processJsonNode function

* updated snapshots tests

* removed unused function

* Better examples (#164)

* changed folder structure

* removed table in Readme of Basic example (not clear and probably also some cells were wrong)

* small update for the Basic example to make it more clear how aliases are referenced

* renamed the “npm” example to “npm module”

* updated “npm” example to use the same config and properties as the “basic” example

* removed license (no sense here) and updated package.json

* updated the s3 example making it more similar to other examples and adding some more assets to be uploaded and linked/embedded in tokens

* updated logo in main Readme in example folder

* updated the Readme for the S3 example

* tried to re-organise the “react” folder in two separate folders

the web app doesn’t compile

* removed spaces from “example” sub-folder

* renamed “example” folder to “examples”

* removed numbers from “examples” sub-folder names

* removed space in sub-folder names

* added advanced example on how to use a watcher to auto-rebuild

see: #171

* small update to Readme for “auto-rebuild-watcher”

* added advanced example on how to have a multi-platform multi-brand suite

* added advanced example on how to use custom templates

* fixed “watch” npm script declaration

* moved packages under “devDependencies” for “custom templates” package

* added a comment in an example of the lodash templating syntax

* remove invisible characters from Readme

* added “clean” npm script call where missing in examples package.json

* added .gitignore file where was missing in examples folder

* updated the config file for the “npm module” example

* added a comment to explain better how the “formatter” function works

* updated the “init” command to expose only the possible/meaningful options + updated documentation for the “examples” page

* added comment about collecting more examples

* updated the Readme for the “examples” folder

* updated “version.js” script as per Danny suggestion

* added advanced example on how to use custom transforms

* updated basic example to use “format” instead of “template” to avoid the alert in console

* added advanced example about referencing/aliasing

* updated example to show reference to an “object-like” value

* removed the advanced examples for react and react native

* added a “create react app” example (with Sass)

* better config for S3 example

* simplified the example for “S3”

* re-introduced android + ios in S3 example

* added a “assets-base64-embed” example

* finalised the “assets-base64-embed” example

* updated Readme for “npm” example + fixed the “prepublishOnly” script option (previous one was deprecated)

* removed the “create-react-app-sass” example (I’ll add it later in a separate ticket)

* updated the documentation

* New cut at documentation PR using current develop branch (#198)

* New cut at documentation PR using current develop branch

* Apply @didoo's suggestions from code review

Co-Authored-By: chazzmoney <[email protected]>

* updates based on didoo's thoughts

* Updating the architecture documentation page (#200)

* updates per didoo and dbanks

* typo

* generation differences

* minor fixes and updates

* making sure sd init command documentation is correct, for now

* updates for clarity around properties and references

* fixing up another alias piece

* Addressing some comments in architecture diagram (#204)

* Final touches on build diagram and architecture (#206)

* Final touches on build diagram and architecture

* Updating build diagram

* Updating build diagram

* Configuration doc update

* fixing snapshot whitespace issues, discovered actual failing test on merge...

* Fixing merge conflict issues

* v2.6.0 release (#210)
@chazzmoney chazzmoney deleted the faster-doc-update branch December 1, 2018 23:17
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