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

Nest service-info under "sequence" and metadata endpoint parameters under "metadata" in refget-openapi.yaml. #332

Merged
merged 1 commit into from
Aug 15, 2018

Conversation

raskoleinonen
Copy link
Contributor

service-info endpoint

Nested under /sequence in the 0.2 specs.

https://github.com/samtools/hts-specs/blob/master/refget.md
Method: Fetch information on the service
GET /sequence/service-info

metadata endpoint

Wrapped within "metadata" object in the 0.2 specs.

@jmarshall
Copy link
Member

Please use specific subject lines in your pull requests. For example, this one could be something like “Fix /sequence/service-info/ in refget-openapi.yaml”. There's no point having a list of PRs that all say the same thing:

image

@raskoleinonen raskoleinonen changed the title Update refget-openapi.yaml Nest service-info under "sequence". Nest metadata endpoint parameters under "metadata". Aug 15, 2018
@raskoleinonen
Copy link
Contributor Author

My apologies. Done.

@raskoleinonen raskoleinonen changed the title Nest service-info under "sequence". Nest metadata endpoint parameters under "metadata". Nest service-info under "sequence" and metadata endpoint parameters under "metadata" in refget-openapi.yaml. Aug 15, 2018
@andrewyatz
Copy link
Contributor

Can you change the first line of your commit message to indicate the change you've made (similar to the name you've given this issue). Sorry otherwise it's just going to appear as Update refget_openapi.yaml

@andrewyatz
Copy link
Contributor

Otherwise the change looks good

@raskoleinonen
Copy link
Contributor Author

It is not straightforward to modify the commit message (https://help.github.com/articles/changing-a-commit-message/). Might be easier if you reject this pull request and I'll just create a new one,

@andrewyatz
Copy link
Contributor

As the GitHub docs say if you can clone your fork of hts-specs then changing the commit is a case of switching to the branch in question and doing the change. In this case I assume you have to do:

git clone https://github.com/raskoleinonen/hts-specs
git checkout patch-3
git commit --amend
# edit the message
git checkout patch-2
# edit the message on the other branches you have
git push --force

This is off the top of my head and also after doing this earlier on today :)

@raskoleinonen raskoleinonen force-pushed the patch-3 branch 2 times, most recently from ff9c3a2 to 6a8dd77 Compare August 15, 2018 15:32
@raskoleinonen
Copy link
Contributor Author

Ok, tried this but managed to change only 1 of 3 commit messages (#330). Clone + checkout + amend for each branch and then git push -force. Tried also checkout + amend + git push -force separately for the two remaining branches. All commit changes have been effected locally but the push was successful only for one (#330). I might be still doing something wrong...

@andrewyatz
Copy link
Contributor

andrewyatz commented Aug 15, 2018 via email

@jmarshall
Copy link
Member

Actually all three have been updated. (It would have been the git push --force separately for each branch that did the trick.) Sometimes you need to refresh the GitHub webpage to see the effects; all three look good via git.

@raskoleinonen
Copy link
Contributor Author

Right, the commit messages updates are now visible in GitHub commit view but the pull request view text remains stubbornly unchanged for this ticket even if I use a different browser etc.

Ok, I'll leave this pull request open in the hope that it might be good enough to be merged.

@andrewyatz andrewyatz merged commit 705b082 into samtools:master Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants