8000 Update plugconf examples function names by hupfdule · Pull Request #271 · vim-volt/volt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Update plugconf examples 8000 function names #271

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
Merged
merged 5 commits into from
Apr 20, 2019
Merged

Update plugconf examples function names #271

merged 5 commits into from
Apr 20, 2019

Conversation

hupfdule
Copy link
Contributor
  • Replaced s:config() with s:on_load_pre() and s:on_load_post()

I noticed a (non-functional) difference between the example in the README and the actual plugconf template: In the README the comments for the plugconf functions are written at the top of the function definition. In the actual template they are witten inside the function defintion.

I actually think putting them on top of the function definition makes more sense, since otherwise they cannnot easily be differentiated to the comments the user adds into the function defintion. Maybe the template should be changed to put the comments on top of the functions?

And another thing: While it makes utter sense to differentiate between configuration that has to be done before loading and configuration that has to be done after loading, the function names s:on_load_pre and s:on_load_post aren't as nice as s:config. s:config makes it clear that it is intended to be used for configuration settings. s:on_load_pre and s:on_load_post actually do make clear when they are being executed, but the intention isn't that clear anymore. Nevertheless, I don't have a suggestion for better names at the moment. :-) Just wanted to bring it to the table. Maybe someone else has an idea if those could functions could be better named.

tyru and others added 3 commits October 27, 2018 07:41
@tyru
Merge pull request vim-volt#229 from vim-volt/devel
0221be8 
Release v0.3.5
@tyru
Merge pull request vim-volt#249 from vim-volt/devel
dd1a16c 
Release v0.3.5a
@hupfdule
Update plugconf examples function names
bf90f38 
- Replaced s:config() with s:on_load_pre() and s:on_load_post()
@hupfdule hupfdule mentioned this pull request Apr 1, 2019
Closed
@tyru
Copy link
Member
tyru commented Apr 18, 2019

Please change target branch to vim-volt:devel .

tyru
tyru requested changes Apr 18, 2019
View reviewed changes
Copy link
Member
@tyru tyru left a comment

Choose a reason for hiding this comment

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

Thanks for the catch-up!

@tyru
Copy link
Member
tyru commented Apr 18, 2019

I noticed a (non-functional) difference between the example in the README and the actual plugconf template: In the README the comments for the plugconf functions are written at the top of the function definition. In the actual template they are witten inside the function defintion.

I actually think putting them on top of the function definition makes more sense, since otherwise they cannnot easily be differentiated to the comments the user adds into the function defintion. Maybe the template should be changed to put the comments on top of the functions?

Thanks for the fix #275 ! 😄

And another thing: While it makes utter sense to differentiate between configuration that has to be done before loading and configuration that has to be done after loading, the function names s:on_load_pre and s:on_load_post aren't as nice as s:config. s:config makes it clear that it is intended to be used for configuration settings. s:on_load_pre and s:on_load_post actually do make clear when they are being executed, but the intention isn't that clear anymore. Nevertheless, I don't have a suggestion for better names at the moment. :-) Just wanted to bring it to the table. Maybe someone else has an idea if those could functions could be better named.

Okay, so could you file an another issue?
We could hear the other person's ideas there.
Personally I slightly agree that the functions are not nice names, but it has already been changed once.
I don't want to easily change the function names in terms of compatibility.

@hupfdule hupfdule changed the base branch from master to devel April 20, 2019 14:26
tyru and others added 2 commits April 20, 2019 16:27
@tyru @hupfdule
Update README.md
0fe7830 
Co-Authored-By: hupfdule <36069345+hupfdule@users.noreply.github.com>
@tyru @hupfdule
Update README.md
9f7ea61 
Co-Authored-By: hupfdule <36069345+hupfdule@users.noreply.github.com>
@tyru tyru merged commit 48376fc into vim-volt:devel Apr 20, 2019
@tyru
Copy link
Member
tyru commented Apr 20, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyru tyru tyru approved these changes

Assignees
No one assigned
Labels
release/v0.3.6
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hupfdule @tyru
0