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

README: Add naming field to example #286

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tadatuta
Copy link
Member

No description provided.

@@ -149,7 +149,8 @@ console.log(configs); // all found configs from all dirs
"levels": [
{
"path": "path/to/level",
"scheme": "nested"
"scheme": "nested",
"naming": "react"
Copy link
Collaborator

@Vittly Vittly Feb 26, 2018

Choose a reason for hiding this comment

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

can I define scheme and naming on the top near the root-prop? Simple example "I want to define react-naming to all my levels"

Copy link
Collaborator

Choose a reason for hiding this comment

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

You promised to write them on the top here ))

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, i misunderstood your idea.
let's doublecheck it again.

for following file structure:

parent-dir/
    .bemrc # root: true here
    project-cwd/
        .bemrc

and

// parent-dir/.bemrc
{
    "root": true,
    "levels": [
        {
            "path": "path/to/level",
            "scheme": "nested",
            "naming": "react"
        }
    ]
}

// parent-dir/project-cwd/.bemrc
{
    "levels": [
        {
            "path": "path/to/level",
            "scheme": "nested"
        }
    ]
}

run

require('@bem/sdk.config')().levelSync('path/to/level'); // { scheme: 'nested', naming: 'react' }

isn't it exactly what you need?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Mmm. levelSync takes path not layer. My bad, sorry

Copy link
Collaborator

@Vittly Vittly Feb 27, 2018

Choose a reason for hiding this comment

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

Will you add naming/scheme props on the top level of config like:

{
  "root": true,
  "naming": "react",
  "levels": ...
}

?

Copy link
Member Author

Choose a reason for hiding this comment

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

It works as you wish right now (any option from root level will be available in levelSync() output). The reason I don't want it in example in readme is that each level can actually have its own naming scheme.

Copy link
Collaborator

@Vittly Vittly Feb 27, 2018

Choose a reason for hiding this comment

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

I understand but specifying naming on top is very common. So I propose to add it in example

@Vittly
Copy link
Collaborator

Vittly commented Mar 12, 2018

@tadatuta lets finish this :)

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.

2 participants