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

Remove old shipment summary worksheet code #1998

Merged
merged 1 commit into from Apr 15, 2019

Conversation

3 participants
@mkrump
Copy link
Contributor

mkrump commented Apr 11, 2019

Description

Delete old / unused code relating to shipment summary worksheet.

Reviewer Notes

@jim it looked like advance_paperwork.go was part of the early ssw work and didn't appear to be used currently. but wanted to confirm that it should be part of cleanup.

Setup

make server_test

Code Review Verification Steps

  • Request review from a member of a different team.
  • Have the Pivotal acceptance criteria been met for this change?

References

@mkrump mkrump added the roci label Apr 11, 2019

@mkrump mkrump requested review from jim and chrisgilmerproj Apr 11, 2019

@@ -265,7 +265,6 @@ build_tools: bash_version server_deps server_generate build_generate_test_data
go build -i -ldflags "$(LDFLAGS)" -o bin/make-dps-user ./cmd/make_dps_user
go build -i -ldflags "$(LDFLAGS)" -o bin/make-office-user ./cmd/make_office_user
go build -i -ldflags "$(LDFLAGS)" -o bin/make-tsp-user ./cmd/make_tsp_user
go build -i -ldflags "$(LDFLAGS)" -o bin/paperwork ./cmd/paperwork

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Apr 11, 2019

Contributor

Do you also need to modify these lines at all?

mymove/Makefile

Lines 218 to 221 in 50ca589

.PHONY: server_go_bindata
server_go_bindata: pkg/assets/assets.go
pkg/assets/assets.go: pkg/paperwork/formtemplates/*
go-bindata -o pkg/assets/assets.go -pkg assets pkg/paperwork/formtemplates/

This comment has been minimized.

Copy link
@mkrump

mkrump Apr 11, 2019

Author Contributor

good point! I actually have't looked at this step before.

I don't think anything needs to be updated here b/c it's only three files in pkg/paperwork/formtemplates/

  • form1203template.png
  • shipment_summary_worksheet_page1.png
  • shipment_summary_worksheet_page2.png

none of which relate to the old stuff.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Apr 11, 2019

Codecov Report

Merging #1998 into master will increase coverage by 0.77%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1998      +/-   ##
==========================================
+ Coverage   60.38%   61.15%   +0.77%     
==========================================
  Files         193      191       -2     
  Lines       12525    12348     -177     
==========================================
- Hits         7563     7551      -12     
+ Misses       4073     3908     -165     
  Partials      889      889
@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

🚀

@jim

This comment has been minimized.

Copy link
Contributor

jim commented Apr 12, 2019

@mkrump I think it's OK to remove advance_paperwork.go as well. We might reuse some of the code that is called from that file, but nothing actually in it.

@jim

jim approved these changes Apr 12, 2019

Copy link
Contributor

jim left a comment

rm that additional file and we're ready to 🚀.

@mkrump

This comment has been minimized.

Copy link
Contributor Author

mkrump commented Apr 12, 2019

@jim I'd already removed advance_paperwork.go in this pr, so let me know if you're pointing to another file.

@mkrump mkrump merged commit b8dc076 into master Apr 15, 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: integration_tests_api 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 3f2a382...50ca589
Details
codecov/project/go 61% (+0.78%) compared to 3f2a382
Details

@mkrump mkrump deleted the mk-164635517-delete-old-ssw-code branch Apr 15, 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.