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

Implement fetching employees via the custom report endpoint #42

Merged
merged 5 commits into from
Mar 1, 2019

Conversation

artfuldodger
Copy link
Contributor

Using the custom report endpoint allows us to get a list of the employees with any fields we want in a much more performant manner than iterating through all the employees and getting additional information one-by-one.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 92.731% when pulling a9be276 on bonusly:custom-reports into 0986b25 on Skookum:master.

@coveralls
Copy link

coveralls commented May 1, 2018

Coverage Status

Coverage increased (+0.7%) to 92.731% when pulling 02bfa9e on bonusly:custom-reports into 0986b25 on Skookum:master.

Copy link

@robertIngrum robertIngrum left a comment

Choose a reason for hiding this comment

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

Definitely seems like a nice improvement!

@splybon splybon changed the base branch from master to develop February 20, 2019 14:27
@splybon
Copy link
Contributor

splybon commented Feb 21, 2019

hey @artfuldodger saw your comment in the other PR and this is looking good. Can you fix some of the rubocop errors here? You can click details in the travis-ci checks, or click the link below

https://travis-ci.org/Skookum/bamboozled/jobs/496020015

@artfuldodger
Copy link
Contributor Author

@splybon I rebased off develop and took care of most of the Rubocop issues. For the remaining issue: I find listing one attribute per line in all_names preferable to having everything on one line (improves, for example, seeing the diff when something changes), so I'm not sure if anything should actually change there. Happy to tweak to match your preferences though.

@splybon
Copy link
Contributor

splybon commented Feb 26, 2019

@artfuldodger I totally agree.

Can you disable that rubocop error then so the linter passes CI?
If you add a comment on or above the method line like below:
# rubocop:disable Metrics/MethodLength

@artfuldodger
Copy link
Contributor Author

@splybon Done! Looks like we're all green now.

@splybon splybon merged commit fb7075d into method-inc:develop Mar 1, 2019
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.

None yet

4 participants