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

add check for results vector matching order of securities vector #126

Merged
merged 1 commit into from Feb 10, 2016

Conversation

@armstrtw
Copy link
Contributor

@armstrtw armstrtw commented Feb 10, 2016

simple string compare to check that the order of the results vector matches what we expect. the new code uses seqNumber from bbg to determine the row number in the results vector. This check just insures that the seqNumber matches the proper position in the securities argument.

johnlaing added a commit that referenced this pull request Feb 10, 2016
add check for results vector matching order of securities vector
@johnlaing johnlaing merged commit e4005b2 into master Feb 10, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/push The Travis CI build passed
Details
@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Feb 10, 2016

Thanks for the PR and merge, looks good.

And the usual plea of pretty please also adding ChangeLog or NEWS.Rd, or else I'll do it :)

@armstrtw armstrtw deleted the add-check-to-bdp branch Feb 10, 2016
@armstrtw
Copy link
Contributor Author

@armstrtw armstrtw commented Feb 11, 2016

do you have a way of producing the changelog from the git log? or do you just edit by hand?

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Feb 11, 2016

If you use the One True Editor Everyone Should Use and hit 'C-x 4 a' then it inserts name, timestamp, file, routine, ... for you. I do not know if vim, eclipse, atom, ... can be taught to be that clever.

And because it is so 'cheap' for me to do it I keep doing it...

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Feb 11, 2016

I looked into a few git2changelog scripts but didn't like any. In practice I have a few nice git aliases here and scribble it from those:

[color]
    ui = true
[alias]
    st = status
    ci = commit
    co = checkout
    ls = log --color --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit
    ll = log --pretty=format:\"%C(yellow)%h%Cred%d %Creset%s%Cblue [%cn]\" --decorate --numstat
    hist = log --graph --decorate --pretty=oneline --abbrev-commit
@armstrtw
Copy link
Contributor Author

@armstrtw armstrtw commented Feb 11, 2016

TH was recently telling us about Magit. He really likes it, but I haven't tried it yet. Not sure if it has something similar built in.

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Feb 11, 2016

I followed up in Slack with a few recommendations...

@armstrtw
Copy link
Contributor Author

@armstrtw armstrtw commented Feb 11, 2016

one question. how do you tell it your name? I was able to figure out email, but not the name it generates.

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Feb 11, 2016

Hold on. I think I set that in ~/.emacs for some things (like the different changelog mode for Debian packages).

Found it in the ~/.emacs at work:

;; make changelog entries complete
(setq user-mail-address "d......@k.........com")
(setq user-full-name "Dirk Eddelbuettel")
@armstrtw
Copy link
Contributor Author

@armstrtw armstrtw commented Feb 12, 2016

thanks!

@armstrtw
Copy link
Contributor Author

@armstrtw armstrtw commented Feb 12, 2016

you want me to make a PR for the changelog or that ok w/ you to just push?

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Feb 12, 2016

I think if it is just ChangeLog you may as well push. And no need for one ChangeLog entry per commit, but one (or more) per PR seems right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.