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

Migrate to using Go Module to manage dependencies #1101

Merged
merged 4 commits into from
Aug 18, 2020

Conversation

huydoan2
Copy link
Contributor

Besides using Go Module (adding go.mod and go.sum), there are several changes:

  1. Upgraded Go version to 1.14 alongside with other packages.
  2. Modified unit tests to accommodate changes.

…h other packages. Modified unit tests to accomodate changes.
@mrutkows
Copy link
Contributor

Appreciate your work on this Huy... hopefully, this a will address the long-standing desire to use go mod as referenced in this issue: #1082

@@ -21,7 +21,6 @@ packages:
hello1:
function: ../src/integration/helloworld/actions/hello.js
limits:
timeout: 1
timeout: 600000
Copy link
Member

Choose a reason for hiding this comment

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

Should this be 60000 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. I didn't change that value. Additionally, that test file is used in parsing tests so the values may not matter much.

Copy link
Contributor

@mrutkows mrutkows Aug 12, 2020

Choose a reason for hiding this comment

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

The value was not intended to be 60 seconds... just testing input matches output against some larger (max) value. Here 10 minutes... The current (default) doc. max is 300000; we could change this, but this is a parser test (no validation is performed at this phase).

@mrutkows
Copy link
Contributor

@style95 Hi Dom, could you and colleagues (who recently showed wskdeploy IDE plugins) please review so we make sure we are not going to break you if we move to merge. Thanks!

@upgle
Copy link
Member

upgle commented Aug 14, 2020

Thank you for your contribution 🎉

Could you please update the README.md? Some contents need to be modified for go mod.
https://github.com/apache/openwhisk-wskdeploy#developers-should-use-go-deps-and-go-build-not-go-get

And I've checked there is no issue for VSCode openwhisk plugin.
@KeonHee Could you check it for Jetbrains extension?

@KeonHee
Copy link
Member

KeonHee commented Aug 14, 2020

@mrutkows @upgle Thanks for noti & @huydoan2 Thanks for your contribution :)

It works well with Jetbrain plugin too.

Copy link
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

Welcome @huydoan2 !
It looks good to me with @upgle's comment. :)

IIRC, @sciabarracom has worked on this as well.
@sciabarracom Do you have any comment?

@huydoan2
Copy link
Contributor Author

Thank you, all.

@upgle We will need to clean up the doc and the Makefile in a separate PR :).

@mrutkows mrutkows merged commit 507665d into apache:master Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants