Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add comments like table columns. #1168

Merged
merged 8 commits into from
Mar 15, 2021
Merged

Add comments like table columns. #1168

merged 8 commits into from
Mar 15, 2021

Conversation

biiiiiigmonster
Copy link
Contributor

@biiiiiigmonster biiiiiigmonster commented Mar 5, 2021

Summary

In order to better support ide, relations and getters/setters can also add comments like table columns. Only based on DocBlock, use @comment.#1165

Support list

  • getter
  • setter
  • scope
  • relations

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

@biiiiiigmonster biiiiiigmonster changed the title add comments like table columns. Add comments like table columns. Mar 5, 2021
Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

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

Despite you ticking off the checkbox, I don't see any tests:
image

Can you please add one before someone reviews the code?
Thank you!

@biiiiiigmonster biiiiiigmonster requested a review from mfn March 6, 2021 01:47
@biiiiiigmonster
Copy link
Contributor Author

I'm sorry that just did a local test, I'll make unit-test it up later.

@mfn mfn removed their request for review March 7, 2021 20:52
@biiiiiigmonster
Copy link
Contributor Author

@mfn I have committed the unit test code, please see if it meets the requirements.

@biiiiiigmonster biiiiiigmonster requested a review from mfn March 9, 2021 01:51
Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

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

What happens if both a getter and a setter has a @comment?

@biiiiiigmonster
Copy link
Contributor Author

I'm not quite sure that you mean, i guess you mean when getter and setter have the same name?

If so, i just tested it, this property will not become two(@property-read&@property-write), but one(@property), and the
comment for this property will be anchoring the getter. If the getter doesn't have @comment, then this property don't either.

The simple code case

class User
{
    /**
     * @comment I'm a setter.
     */
    public function setFullNameAttribute(): void
    {}

    /**
     * @comment I'm a getter.(If I don't exist, so will the property)
     */
    public function getFullNameAttribute(): string
    {}
}

// =>

/**
 * @property string $full_name I'm a getter.(If I don't exist, so will the property)
 * …
 */

@biiiiiigmonster biiiiiigmonster requested a review from mfn March 13, 2021 07:09
Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

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

LGTM!

  • I added a suggestion to improve the README
  • change you please add a changelog entry?

@mfn mfn requested a review from barryvdh March 13, 2021 19:40
biiiiiigmonster and others added 2 commits March 14, 2021 21:00
improve README

Co-authored-by: Markus Podar <markus@fischer.name>
@biiiiiigmonster
Copy link
Contributor Author

OK, i got it :)

@barryvdh barryvdh merged commit c5c9b2b into barryvdh:master Mar 15, 2021
kodiakhq bot referenced this pull request in WesleyKlop/vote-system Mar 16, 2021
Bumps [barryvdh/laravel-ide-helper](https://github.com/barryvdh/laravel-ide-helper) from 2.9.0 to 2.9.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/barryvdh/laravel-ide-helper/releases">barryvdh/laravel-ide-helper's releases</a>.</em></p>
<blockquote>
<h2>v2.9.1</h2>
<h3>Added</h3>
<ul>
<li>Generate PHPDoc for Laravel 8.x factories <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1074">#1074 / ahmed-aliraqi</a></li>
<li>Add a comment to a property like table columns <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1168">#1168 / biiiiiigmonster</a></li>
<li>Added <code>post_migrate</code> hook to run commands after a migration <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1163">#1163 / netpol</a></li>
</ul>
<h3>Fixed</h3>
<ul>
<li>Error when generating helper for invokable classes <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1124">#1124 / standaniels</a></li>
<li>Fix broken ReflectionUnionTypes <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1132">#1132 / def-studio</a></li>
<li>Relative class names are not converted to fully-qualified class names <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1005">#1005 / SavKS</a></li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/barryvdh/laravel-ide-helper/blob/master/CHANGELOG.md">barryvdh/laravel-ide-helper's changelog</a>.</em></p>
<blockquote>
<h2>2021-03-15, 2.9.1</h2>
<h3>Added</h3>
<ul>
<li>Generate PHPDoc for Laravel 8.x factories <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1074">#1074 / ahmed-aliraqi</a></li>
<li>Add a comment to a property like table columns <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1168">#1168 / biiiiiigmonster</a></li>
<li>Added <code>post_migrate</code> hook to run commands after a migration <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1163">#1163 / netpol</a></li>
</ul>
<h3>Fixed</h3>
<ul>
<li>Error when generating helper for invokable classes <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1124">#1124 / standaniels</a></li>
<li>Fix broken ReflectionUnionTypes <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1132">#1132 / def-studio</a></li>
<li>Relative class names are not converted to fully-qualified class names <a href="https://github.com/barryvdh/laravel-ide-helper/pull/1005">#1005 / SavKS</a></li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/8d8302ff6adb55f8b844c798b8b1ffdee142f7e5"><code>8d8302f</code></a> chore: add missing changelog entries (<a href="https://github.com/barryvdh/laravel-ide-helper/issues/1176">#1176</a>)</li>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/202395b50f31325911fd045993c2b4d2851b8bf6"><code>202395b</code></a> Auto generate models (<a href="https://github.com/barryvdh/laravel-ide-helper/issues/1163">#1163</a>)</li>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/c5c9b2b9517696e1d17ce64790be42b8bb76a4d2"><code>c5c9b2b</code></a> Add comments like table columns. (<a href="https://github.com/barryvdh/laravel-ide-helper/issues/1168">#1168</a>)</li>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/c5e18beff1a2933913f86d56281581566e5c8705"><code>c5e18be</code></a> Fix Relative class names are not converted to fully-qualified class names (FQ...</li>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/6c4dcd9049e7e9aabc78673de7d1b9d428f3dc10"><code>6c4dcd9</code></a> Merge pull request <a href="https://github.com/barryvdh/laravel-ide-helper/issues/1164">#1164</a> from wilsenhc/readme-fix</li>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/aaade4eab932efe3841306ad7faf9ef1a8cef391"><code>aaade4e</code></a> Fix typo in README</li>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/b21324b20e24a3c36bedc184b4cf273bfa0a2d32"><code>b21324b</code></a> Merge pull request <a href="https://github.com/barryvdh/laravel-ide-helper/issues/1156">#1156</a> from ZaidBarghouthi/fix-zb-extra-quote</li>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/db83fb8b6ba38ea0b1d28197e898b4ff0d9581d0"><code>db83fb8</code></a> Remove extra single quote</li>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/4f42af1fa1f6265174ef97c36e0ae7beb2596c29"><code>4f42af1</code></a> Apply &quot;<a href="https://github.com/PSR12"><code>@​PSR12</code></a>&quot; in php-cs-fixer (<a href="https://github.com/barryvdh/laravel-ide-helper/issues/1150">#1150</a>)</li>
<li><a href="https://github.com/barryvdh/laravel-ide-helper/commit/f0959c1184c6f8f9e50afa6d9b6c1eb7691ae3ae"><code>f0959c1</code></a> Update changelog and also automagically apply style fix (<a href="https://github.com/barryvdh/laravel-ide-helper/issues/1140">#1140</a>)</li>
<li>Additional commits viewable in <a href="https://github.com/barryvdh/laravel-ide-helper/compare/v2.9.0...v2.9.1">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=barryvdh/laravel-ide-helper&package-manager=composer&previous-version=2.9.0&new-version=2.9.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants