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

Plugin support #1075

Merged
merged 123 commits into from Oct 5, 2016

Conversation

Projects
4 participants
@borekb
Member

borekb commented Jun 1, 2016

Resolves most of #1036.

Progress: #1075 (comment)

Update 4th Oct 2016: We're mostly done with this, some smaller things have been moved to separate issues and are linked from #1075 (comment). If you want to provide feedback, please read Plugin-Support.md and leave a comment.

borekb and others added some commits Jun 1, 2016

@JanVoracek JanVoracek self-assigned this Jun 3, 2016

@JanVoracek JanVoracek added this to the 4.0 milestone Jun 3, 2016

@vasek17 vasek17 closed this Jul 11, 2016

@vasek17 vasek17 deleted the 1036-plugin-support branch Jul 11, 2016

@JanVoracek JanVoracek restored the 1036-plugin-support branch Jul 11, 2016

@JanVoracek

This comment has been minimized.

Show comment
Hide comment
@JanVoracek

JanVoracek Jul 11, 2016

Member

Closed by mistake; reopening.

Member

JanVoracek commented Jul 11, 2016

Closed by mistake; reopening.

@JanVoracek JanVoracek reopened this Jul 11, 2016

JanVoracek added some commits Jul 12, 2016

[#1036] Removed all custom entity-related *ChangeInfo classes and all…
… custom Bulk*ChangeInfo classes

Note: This commit will not pass the tests. I haven't found any easy way how to do this change in small steps. There remains a lot of related changes but is already too big; thus, I will commit it in multiple commits.
[#1036] ChangeInfo objects are now parsed from commit messages using …
…`ChangeInfoFactory` instead of many `buildFromCommitMessage` methods

JanVoracek and others added some commits Sep 21, 2016

[#1036] First pass of Plugin-Support.md review. Notable changes:
- Actions moved as the top section
- ... and quite heavily edited
- `.versionpress` folder mentioned more explicitly
- Trying out a new term "plugin descriptor".
[#1036] Made heading structure one level flatter – actions, DB schema…
… etc. are now H2 instead of H3. This allows easier structuring inside them.
@borekb

This comment has been minimized.

Show comment
Hide comment
@borekb

borekb Oct 3, 2016

Member

Here are some notes as I reviewed Plugin-Support.md:

  • It would be good to have a clear name for the set of files in the .versionpress folder. "Plugin definition" is the best I can currently think of.
    • Yes let's go with "plugin definition", that's fine.
  • We have been using the edit action since VersionPress 1.0 (e.g., option/edit), however, the more natural English verb is update. 4.0 would be a chance to fix this. (Feedback from native speakers welcome.)
    • We agreed that update is more natural. #1120
  • Do we need the separate _meta_ entity filters, e.g., vp_entity_tags_ vs. vp_meta_entity_tags_? I get that they are slightly different, however, if a single filter name could be used for both cases, it would make the API smaller.
  • Is "schema" the right word? Our schema.yml contains quite a bit more. On the other hand, "schema" is quite unique and intuitively connected to a database so it might be OK to call it that.
    • We could not come up with a better alternative.
  • .versionpress makes sense in the plugin folder but for the planned online repo, we need a convention to separate the plugins. See e.g. DefinitelyTyped.
    • Something like plugin's slug will be used.
  • Our term "actions" conflicts with WP actions. Not be a big deal for us but WP developers unfamiliar with VersionPress might be confused. If needs be, we can consider "events" instead.
    • As nouns the difference between action and event is that action is something done so as to accomplish a purpose while event is an occurrence; something that happens. -- http://wikidiff.com/event/action

    • In the team, we prefer actions over events (quite obviously as we've used that in the past). Hopefully it's ok for WP devs.
  • Document the slash in Actions
Member

borekb commented Oct 3, 2016

Here are some notes as I reviewed Plugin-Support.md:

  • It would be good to have a clear name for the set of files in the .versionpress folder. "Plugin definition" is the best I can currently think of.
    • Yes let's go with "plugin definition", that's fine.
  • We have been using the edit action since VersionPress 1.0 (e.g., option/edit), however, the more natural English verb is update. 4.0 would be a chance to fix this. (Feedback from native speakers welcome.)
    • We agreed that update is more natural. #1120
  • Do we need the separate _meta_ entity filters, e.g., vp_entity_tags_ vs. vp_meta_entity_tags_? I get that they are slightly different, however, if a single filter name could be used for both cases, it would make the API smaller.
  • Is "schema" the right word? Our schema.yml contains quite a bit more. On the other hand, "schema" is quite unique and intuitively connected to a database so it might be OK to call it that.
    • We could not come up with a better alternative.
  • .versionpress makes sense in the plugin folder but for the planned online repo, we need a convention to separate the plugins. See e.g. DefinitelyTyped.
    • Something like plugin's slug will be used.
  • Our term "actions" conflicts with WP actions. Not be a big deal for us but WP developers unfamiliar with VersionPress might be confused. If needs be, we can consider "events" instead.
    • As nouns the difference between action and event is that action is something done so as to accomplish a purpose while event is an occurrence; something that happens. -- http://wikidiff.com/event/action

    • In the team, we prefer actions over events (quite obviously as we've used that in the past). Hopefully it's ok for WP devs.
  • Document the slash in Actions
- Hooks are defined in `hooks.php`
All files are optional so for example, if a plugin doesn't define any new shortcodes it can omit the `shortcodes.yml` file. Simple plugins like _Hello Dolly_ might even omit everything; they just need to have the `.versionpress` folder so that VersionPress knows the plugin is supported.

This comment has been minimized.

@octopuss

octopuss Oct 3, 2016

Contributor

How will VP versioning mechanism handle this Empty folder ? Empty folders are not versioned by default.

@octopuss

octopuss Oct 3, 2016

Contributor

How will VP versioning mechanism handle this Empty folder ? Empty folders are not versioned by default.

This comment has been minimized.

@JanVoracek

JanVoracek Oct 3, 2016

Member

I don't really know. Originally, I didn't intend to use this directory to let VP know if the plugin is supported. It's maybe question to @borekb.

@JanVoracek

JanVoracek Oct 3, 2016

Member

I don't really know. Originally, I didn't intend to use this directory to let VP know if the plugin is supported. It's maybe question to @borekb.

This comment has been minimized.

@borekb

borekb Oct 3, 2016

Member

.gitkeep is an option but it's true that we didn't discuss this yet. The empty .versionpress folder seemed to me like the easiest way for a plugin author to declare explicit support for VersionPress even if no definitions are necessary.

Or maybe it could be a .versionpress file?

@borekb

borekb Oct 3, 2016

Member

.gitkeep is an option but it's true that we didn't discuss this yet. The empty .versionpress folder seemed to me like the easiest way for a plugin author to declare explicit support for VersionPress even if no definitions are necessary.

Or maybe it could be a .versionpress file?

This comment has been minimized.

@JanVoracek

JanVoracek Oct 4, 2016

Member

I think it should be some service. The main reason is that you don't have the files before you install the plugin; therefore, you cannot say whether the plugin is supported or not on the /plugin-install.php page.

@JanVoracek

JanVoracek Oct 4, 2016

Member

I think it should be some service. The main reason is that you don't have the files before you install the plugin; therefore, you cannot say whether the plugin is supported or not on the /plugin-install.php page.

This comment has been minimized.

@borekb

borekb Oct 4, 2016

Member

Good point. Let's talk about this later, seems like a relative detail.

@borekb

borekb Oct 4, 2016

Member

Good point. Let's talk about this later, seems like a relative detail.

```yaml
post:
table: posts
id: ID

This comment has been minimized.

@octopuss

octopuss Oct 3, 2016

Contributor

What about multi column id ?

@octopuss

octopuss Oct 3, 2016

Contributor

What about multi column id ?

This comment has been minimized.

@JanVoracek

JanVoracek Oct 3, 2016

Member

VersionPress doesn't support it yet.

@JanVoracek

JanVoracek Oct 3, 2016

Member

VersionPress doesn't support it yet.

Show outdated Hide outdated docs/Plugin-Support.md
### Non-database actions
Some actions are not directly related to the database entities, e.g. plugin installation, WP update, etc. VersionPress provides function `vp_force_action` for these actions. VersionPress will use only action specified by parameters of this function and ignore all automatically catched. For example:

This comment has been minimized.

@octopuss

octopuss Oct 3, 2016

Contributor

Add note that priority is ignored when action is forced.

@octopuss

octopuss Oct 3, 2016

Contributor

Add note that priority is ignored when action is forced.

This comment has been minimized.

@JanVoracek

JanVoracek Oct 3, 2016

Member

Not only the priority – the actions are ignored entirely. Do you have any suggestion how to reword the sentence?

@JanVoracek

JanVoracek Oct 3, 2016

Member

Not only the priority – the actions are ignored entirely. Do you have any suggestion how to reword the sentence?

This comment has been minimized.

@borekb

borekb Oct 3, 2016

Member

I wanted to look at this section anyway ("catched" etc.), will try to reword it.

@borekb

borekb Oct 3, 2016

Member

I wanted to look at this section anyway ("catched" etc.), will try to reword it.

This comment has been minimized.

@JanVoracek

JanVoracek Oct 4, 2016

Member

I need an emoji for :facepalm: ... catched 🙄

@JanVoracek

JanVoracek Oct 4, 2016

Member

I need an emoji for :facepalm: ... catched 🙄

{
return $this->commitMessage->getUnprefixedSubject();
return 0;

This comment has been minimized.

@octopuss

octopuss Oct 3, 2016

Contributor

When no better is found, why priority is not high instead ?

@octopuss

octopuss Oct 3, 2016

Contributor

When no better is found, why priority is not high instead ?

This comment has been minimized.

@JanVoracek

JanVoracek Oct 3, 2016

Member

UntrackedChangeInfo represents manual commit; thus, there are no other ChangeInfo objects to compare with. Plus, ignoring the fact it will not compare with anything, zero is the highest priority.

@JanVoracek

JanVoracek Oct 3, 2016

Member

UntrackedChangeInfo represents manual commit; thus, there are no other ChangeInfo objects to compare with. Plus, ignoring the fact it will not compare with anything, zero is the highest priority.

borekb and others added some commits Oct 3, 2016

[#1036] I've simplified and rearranged the section on actions. Details:
- Removed section about how VersionPress stores actions in commit messages (it didn't bring enough value for its length)
- More clearly described what an action is and what it consists of
- There is now clearer distinction between database and non-database actions
- Filters are just mentioned but their signature is not described
- Some other smaller edits
Merge branch 'master' into 1036-plugin-support
# Conflicts:
#	plugins/versionpress/composer.lock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment