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

Some .gitignore cleanups for binaries #1825

Merged
merged 2 commits into from Mar 7, 2019

Conversation

3 participants
@reggieriser
Copy link
Contributor

reggieriser commented Mar 6, 2019

Description

I noticed that I had an untracked file for bin/save-fuel-price-data (the code for that command got merged yesterday). This PR adds a .gitignore for that, but I also went through and audited our bin directory against the Makefile and removed bin/rateengine (that command was deleted a while back) and bin/golint (think that's old too). While I was at it, I re-alphabetized the bin stuff in .gitignore and the build_tools Makefile target.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Mar 6, 2019

Codecov Report

Merging #1825 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1825   +/-   ##
=======================================
  Coverage   49.42%   49.42%           
=======================================
  Files         427      427           
  Lines       18328    18328           
  Branches     1631     1631           
=======================================
  Hits         9058     9058           
  Misses       8473     8473           
  Partials      797      797
@kimallen

This comment has been minimized.

Copy link
Contributor

kimallen commented Mar 6, 2019

Thanks for doing the cleanup! Was adding it to the .gitignore something I should have done for my PR? Should all the commands be listed in the Makefile under build_tools? I noticed that a couple others that I've created are not in there: generate_shipment_edi and send_to_gex. If not, when should they be listed and when should they not?

@reggieriser

This comment has been minimized.

Copy link
Contributor Author

reggieriser commented Mar 6, 2019

Since the build_tools added the save-fuel-price-data command, we should probably have added a .gitignore for it too (I missed that too in the review!). No big deal.

As far as the other tools you mentioned, I guess it depends on whether we want to always build them alongside the other tools. Those are mainly debugging/testing tools, right? So maybe we just build them on demand. @chrisgilmerproj or @pjdufour-truss , thoughts?

@chrisgilmerproj

This comment has been minimized.

Copy link
Contributor

chrisgilmerproj commented Mar 6, 2019

I don't mind building all the tools all the time. Some tools are called out with separate build paths because of their need to be built individually for CI/CD, but I'd say we should build everything we can in order to catch bugs early when code changes.

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

Let's add the other tools and then I'm happy with this. Thanks! (I especially appreciate the alphabetization). We should probably follow up and ensure that the .dockerignore has most of the same stuff as the .gitignore, but that can wait.

@reggieriser reggieriser merged commit 38ce009 into master Mar 7, 2019

19 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: client_test_coverage Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_mymove Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_office Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_tsp Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
ci/circleci: server_test_coverage Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 0a60bb0...fbb00de
Details
codecov/project 49.42% remains the same compared to 0a60bb0
Details

@reggieriser reggieriser deleted the rr-gitignore-cleanup branch Mar 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.