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

feature: template function #53

Merged
merged 3 commits into from
Aug 24, 2020
Merged

feature: template function #53

merged 3 commits into from
Aug 24, 2020

Conversation

ak1ra24
Copy link
Collaborator

@ak1ra24 ak1ra24 commented Aug 23, 2020

#52

@ak1ra24 ak1ra24 requested a review from slankdev as a code owner August 23, 2020 16:11
@ak1ra24 ak1ra24 marked this pull request as draft August 23, 2020 16:21
@ak1ra24 ak1ra24 marked this pull request as ready for review August 23, 2020 16:32
Comment on lines +84 to +88
type Template struct {
Src string `yaml:"src"`
Dst string `yaml:"dst"`
Content string `yaml:"content"`
}
Copy link
Contributor

Choose a reason for hiding this comment

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

これもだだの今後の議論ポイントとして.

もしかすると, 以下のようでもよかったかもしれないね.
両方(content, src) が両方選択されると, 片方は無視されるだけだけど, 指定されたかどうかとかは, mapとしてはpointer使ってその辺を整理出来るようにする方法もあると思う.

今は十分だと思うからこれでいいけど. ポイントとして.

type Template struct {
  Src *string
  Dst *string
  Content *string
}

Copy link
Contributor

@slankdev slankdev left a comment

Choose a reason for hiding this comment

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

一つだけ. 他はこのままでいいと思う.

Copy link
Contributor

@slankdev slankdev left a comment

Choose a reason for hiding this comment

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

1つだけ.
これ以外は問題ないと思う.

internal/pkg/shell/shell.go Outdated Show resolved Hide resolved
@slankdev
Copy link
Contributor

全然関係ないけどPRごとに, buildしてbinaryをこのconversationに添付するロボットがほしい.

Copy link
Contributor

@slankdev slankdev left a comment

Choose a reason for hiding this comment

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

もう一つあった.
と思ったけど.

少なくとも os.Openは使わないといけないのか... 仕方ないか.

internal/pkg/shell/shell.go Show resolved Hide resolved
@github-actions github-actions bot requested a review from slankdev August 24, 2020 03:44
Copy link
Contributor

@slankdev slankdev left a comment

Choose a reason for hiding this comment

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

LGTM

@slankdev slankdev merged commit b5157a1 into master Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants