Skip to content

WIP: refactor(*): return FileHeader in the File method #436

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.

Sign up for GitHub

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

Jump to bottom
Open
iawia002 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP: refactor(*): return FileHeader in the File method #436

iawia002 wants to merge 1 commit into from

Conversation

@iawia002
Copy link

@iawia002 iawia002 commented Jan 12, 2021

What this PR does / why we need it:

Add your description

Which issue(s) this PR is related to (optional, link to 3rd issue(s)):

Fixes #

Reference to #

Special notes for your reviewer:

/cc @whalecold

Release note:

NONE

@caicloud-bot caicloud-bot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 12, 2021
@caicloud-bot
Copy link
Contributor

caicloud-bot commented Jan 12, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: ddysher

Assign the PR to them by writing /assign @ddysher in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

whalecold
whalecold reviewed Jan 12, 2021
View reviewed changes
service/source.go
func (g *FileParameterGenerator) Generate(ctx context.Context, vc ValueContainer, consumers []Consumer,
name string, target reflect.Type) (interface{}, error) {
file, ok := vc.File(name)
file, _, ok := vc.File(name)
Copy link
Member

@whalecold whalecold Jan 12, 2021

Choose a reason for hiding this comment

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

这样的话我怎么才能拿到这个 header

Copy link
Author

@iawia002 iawia002 Jan 12, 2021

Choose a reason for hiding this comment

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

你想从业务函数那里直接拿到吗，那里是拿不到的，那儿只有一个位置，也不可能对应两个参数

service.HTTPContextFrom(ctx) 转换 context 可以取到这个值

Copy link
Member

@whalecold whalecold Jan 12, 2021

Choose a reason for hiding this comment

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

那是不是返回 FileHeader 更好，因为 File 可以通过 FileHeader.Open() 拿到

@iawia002 iawia002 changed the title refactor(*): return FileHeader in the File method WIP: refactor(*): return FileHeader in the File method Jan 12, 2021
@caicloud-bot caicloud-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@whalecold whalecold whalecold left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iawia002 @caicloud-bot @whalecold