Skip to content

Note in README that substitutions don’t work inside quotes#634

Merged
2m merged 2 commits intomasterfrom
havocp-readme-quotes
May 23, 2019
Merged

Note in README that substitutions don’t work inside quotes#634
2m merged 2 commits intomasterfrom
havocp-readme-quotes

Conversation

@havocp
Copy link
Contributor

@havocp havocp commented May 21, 2019

There’s more on this in HOCON.md but can’t hurt to repeat, people do miss it.

@havocp
Note in README that substitutions don’t work inside quotes
8960c92 
There’s more on this in HOCON.md but can’t hurt to repeat, people do miss it.
@havocp havocp mentioned this pull request May 21, 2019
Closed
TimMoore
TimMoore reviewed May 21, 2019
View reviewed changes
Copy link

@TimMoore TimMoore left a comment

Choose a reason for hiding this comment

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

Thanks! It can't hurt to make it more obvious 👍

README.md Outdated
Quoted or unquoted strings can also concatenate with substitutions of course:

tasks-url : ${base-url}/tasks
tasks-url : ${base-url}”tasks:colon-must-be-quoted”
Copy link

@TimMoore TimMoore May 21, 2019

Choose a reason for hiding this comment

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

It looks like some "smart" (curly) quotes snuck in.

Copy link

@TimMoore TimMoore May 21, 2019

Choose a reason for hiding this comment

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

Suggested change
tasks-url : ${base-url}”tasks:colon-must-be-quoted”
tasks-url : ${base-url}"tasks:colon-must-be-quoted"

Copy link
Contributor Author

@havocp havocp May 23, 2019

Choose a reason for hiding this comment

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

Thanks, apparently I shouldn't make PRs on an iPhone. Amusingly, the smart quotes appear to crash github's "commit suggestion" button so I have to fix this from my laptop ...

image

@havocp
Copy link
Contributor Author

havocp commented May 23, 2019

@2m when I try to push another commit to this branch it says:

remote: error: GH006: Protected branch update failed for refs/heads/havocp-readme-quotes.
remote: error: Required status check "typesafe-cla-validator" is expected.
To github.com:lightbend/config.git
 ! [remote rejected] havocp-readme-quotes -> havocp-readme-quotes (protected branch hook declined)
error: failed to push some refs to 'git@github.com:lightbend/config.git'

Maybe I'm half asleep but I don't know what's going wrong there. I re-clicked the CLA to be sure but note the original PR has the CLA check passed.

@2m
Copy link
Contributor

2m commented May 23, 2019

Its the first time I've seen branch protection rule fail because of CLA validator when pushing an additional commit to the branch. Especially since CLA validator kicks in after the commit has been pushed.

To side-step this we could only set the master branch as the protected one. Currently all of the branches are protected in this repo.

@havocp
Copy link
Contributor Author

havocp commented May 23, 2019

Protecting only master is fine with me if you want to make that change (I don't think I have access). It seems like there's a bug here but not sure if it's in the cla-validator or in GitHub or something I'm doing wrong.

@2m
Copy link
Contributor

2m commented May 23, 2019

Alright, changed to only protect master.

@2m 2m merged commit 8fc33c8 into master May 23, 2019
@2m 2m deleted the havocp-readme-quotes branch May 23, 2019 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@TimMoore TimMoore TimMoore left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@havocp @2m @TimMoore