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

sync-template.sh script does not create pull request #14

Closed
i386x opened this issue Aug 26, 2020 · 3 comments
Closed

sync-template.sh script does not create pull request #14

i386x opened this issue Aug 26, 2020 · 3 comments

Comments

@i386x
Copy link
Contributor

i386x commented Aug 26, 2020

I just run ./sync-template.sh -r kernel_settings -t {{token}} and I don't see anything in https://github.com/linux-system-roles/kernel_settings/pulls. By investigating the script's output, I found

{
  "message": "Problems parsing JSON",
  "documentation_url": "https://docs.github.com/rest/reference/pulls#create-a-pull-request"
}

This is related to curl command, which was run as

curl -u 'systemroller:{{token}}' -X POST -d '{"title":"Synchronize files from linux-system-roles/template",
"base":"master",
"head":"lsr-template-sync",
"body":"This PR propagates files from [linux-system-roles/template](https://github.com/linux-system-roles/template) which should be in sync across [linux-system-roles](https://github.com/linux-system-roles) repos. In case of changing affected files via pushing to this PR, please do not forget also to push the changes to [linux-system-roles/template](https://github.com/linux-system-roles/template) repo.

Revision: [`448ff39c16f171b7f09ff5a21e62340add304d55`](https://github.com/linux-system-roles/template/tree/448ff39c16f171b7f09ff5a21e62340add304d55)

CC: @i386x, @pcahyna, @richm"}' 'https://api.github.com/repos/linux-system-roles/kernel_settings/pulls'

curl command above exits with 0, so the script behaves like everything have been ok.

Observe that commit linux-system-roles/kernel_settings@54d04c1 has been pushed to lsr-template-sync branch successfully.

@richm
Copy link
Contributor

richm commented Aug 26, 2020

Hmm - seems that JSON does not like the newline characters in the body of the PR message:

{"title":"Synchronize files from linux-system-roles/template",
"base":"master",
"head":"lsr-template-sync",
"body":"This PR propagates files from [linux-system-roles/template](https://gith
ub.com/linux-system-roles/template) which should be in sync across [linux-system
-roles](https://github.com/linux-system-roles) repos. In case of changing affect
ed files via pushing to this PR, please do not forget also to push the changes t
o [linux-system-roles/template](https://github.com/linux-system-roles/template) 
repo.

Revision: [`448ff39c16f171b7f09ff5a21e62340add304d55`](https://github.com/linux-
system-roles/template/tree/448ff39c16f171b7f09ff5a21e62340add304d55)

CC: @i386x, @pcahyna, @richm"}

feed this to jq:

parse error: Invalid string: control characters from U+0000 through U+001F must be escaped at line 8, column 29

remove the newlines from the "body" field:

{"title":"Synchronize files from linux-system-roles/template",
"base":"master",
"head":"lsr-template-sync",
"body":"This PR propagates files from [linux-system-roles/template](https://gith
ub.com/linux-system-roles/template) which should be in sync across [linux-system
-roles](https://github.com/linux-system-roles) repos. In case of changing affect
ed files via pushing to this PR, please do not forget also to push the changes t
o [linux-system-roles/template](https://github.com/linux-system-roles/template) 
repo. Revision: [`448ff39c16f171b7f09ff5a21e62340add304d55`](https://github.com/
linux-system-roles/template/tree/448ff39c16f171b7f09ff5a21e62340add304d55) CC: @
i386x, @pcahyna, @richm"}

feed this to jq

{
  "title": "Synchronize files from linux-system-roles/template",
  "base": "master",
  "head": "lsr-template-sync",
  "body": "This PR propagates files from [linux-system-roles/template](https://github.com/linux-system-roles/template) which should be in sync across [linux-system-roles](https://github.com/linux-system-roles) repos. In case of changing affected files via pushing to this PR, please do not forget also to push the changes to [linux-system-roles/template](https://github.com/linux-system-roles/template) repo. Revision: [`448ff39c16f171b7f09ff5a21e62340add304d55`](https://github.com/linux-system-roles/template/tree/448ff39c16f171b7f09ff5a21e62340add304d55) CC: @i386x, @pcahyna, @richm"
}

I probably broke this when I added support for hub to the sync script. That is, I probably broke the newline escaping.

@i386x
Copy link
Contributor Author

i386x commented Aug 26, 2020

Bash probably transforms escaped \n to real \n when payload string was passed to submit_pr. That's why I don't like strings in Bash. We need to setup unit tests and CI for it with curl, git, and hub mocked.

@richm
Copy link
Contributor

richm commented Jun 10, 2021

obsolete since we use tox-lsr now

@richm richm closed this as completed Jun 10, 2021
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

No branches or pull requests

2 participants