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

Do not overwrite go.mod on make if it exists. #7245

Merged
merged 1 commit into from
Jan 22, 2020
Merged

Do not overwrite go.mod on make if it exists. #7245

merged 1 commit into from
Jan 22, 2020

Conversation

ro-tex
Copy link
Contributor

@ro-tex ro-tex commented Jan 21, 2020

What did you implement

Ensures that running make does not overwrite go.mod if it already exists.

Closes #7244

How can we verify it

This is my serverless.yml:

service: golambda
app: go-lambda

frameworkVersion: '>=1.28.0 <2.0.0'

provider:
  name: aws
  runtime: go1.x
  region: eu-west-1

package:
  exclude:
    - ./**
  include:
    - ./bin/**

functions:
  hello:
    handler: bin/hello
    events:
      - http:
          path: hello
          method: get
  docs:
    handler: bin/docs
    environment:
      JWT_SECRET: ${env:JWT_SECRET}
    events:
      - http:
          path: v0/docs
          method: get
      - http:
          path: v0/docs
          method: post
      - http:
          path: v0/docs/{id}
          method: any

plugins:
  - serverless-dotenv-plugin
  1. You can run this without any actual go code if you delete all function definitions - this will not change the relevant execution.
  2. Run make in order to generate your go.mod file.
  3. Edit go.mod - add a dependency, comment, etc.
  4. Run make again in order to rebuild your binaries.
  5. Verify that your changes to go.mod are still present.

A project using this fix: https://gitlab.com/ro-tex/golambda

Todos

Useful Scripts
  • npm run test:ci --> Run all validation checks on proposed changes
  • npm run lint:updated --> Lint all the updated files
  • npm run lint:fix --> Automatically fix lint problems (if possible)
  • npm run prettier-check:updated --> Check if updated files adhere to Prettier config
  • npm run prettify:updated --> Prettify all the updated files
  • [ x] Write and run all tests
  • Write documentation
  • [ x] Enable "Allow edits from maintainers" for this PR
  • [ x] Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@codecov-io
Copy link

Codecov Report

Merging #7245 into master will decrease coverage by 0.33%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7245      +/-   ##
==========================================
- Coverage   88.48%   88.14%   -0.34%     
==========================================
  Files         229      236       +7     
  Lines        8418     8641     +223     
==========================================
+ Hits         7449     7617     +168     
- Misses        969     1024      +55
Impacted Files Coverage Δ
...ckage/compile/events/apiGateway/lib/permissions.js 87.5% <0%> (-5.36%) ⬇️
lib/plugins/package/lib/packageService.js 86.06% <0%> (-1.44%) ⬇️
lib/utils/config/index.js 73.07% <0%> (-1.44%) ⬇️
lib/plugins/aws/lib/naming.js 97.81% <0%> (-1.41%) ⬇️
lib/plugins/package/lib/zipService.js 95.49% <0%> (-0.84%) ⬇️
lib/plugins/plugin/lib/utils.js 97.36% <0%> (-0.68%) ⬇️
lib/classes/CLI.js 95.18% <0%> (-0.55%) ⬇️
lib/classes/Variables.js 99.72% <0%> (-0.28%) ⬇️
lib/plugins/aws/customResources/index.js 98.59% <0%> (-0.08%) ⬇️
...kage/compile/events/apiGateway/lib/method/index.js 100% <0%> (ø) ⬆️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1d2d00...583febe. Read the comment docs.

Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Thank you @ro-tex ! Looks good to me

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.

gomod.sh overwrites go.mod if it exists
3 participants