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

Added app/Common.php #2110

Merged
merged 1 commit into from
Aug 28, 2019
Merged

Added app/Common.php #2110

merged 1 commit into from
Aug 28, 2019

Conversation

jason-napolitano
Copy link
Contributor

@jason-napolitano jason-napolitano commented Jul 28, 2019

A commit that will hopefully help with this topic: #2101. This is a re-submit due to lack of GPG signing in the previous PR: #2105

Each pull request should address a single issue, and have a meaningful title.

Description
Explain what you have changed, and why.

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

---------Remove from here down in your description----------

Notes

  • Pull requests must be in English
  • If the PR solves an issue, reference it with a suitable verb and the issue number
    (e.g. fixes 12345
  • Unsolicited PRs will be considered, but there is no guarantee of acceptance
  • Pull requests should be from a feature branch in the contributor's fork of the repository
    to the develop branch of the project repository

A commit that will hopefully help with this topic: #2101. This is a re-submit due to lack of GPG signing in the previous PR: #2105
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Typo on first comment: “Te goal”. Also still need the bracket line break:

if ...
{

This will definitely need User Guide updates, probably in the Extending section, but I can work on that since you said you were unfamiliar with the doc format (if the admins are willing to approve without it).

@MGatner
Copy link
Member

MGatner commented Aug 13, 2019

@jason-napolitano Can we get that typo fixed, and the newlines for brackets?

@lonnieezell Do you approve of this approach? I can update docs for it if so.

@lonnieezell
Copy link
Member

@MGatner Yes, I approve. We had discussed this previously and then it got lost in the shuffle of me moving.

@MGatner MGatner mentioned this pull request Aug 25, 2019
5 tasks
@MGatner
Copy link
Member

MGatner commented Aug 26, 2019

Okay got the docs in on another PR. @jason-napolitano I would still like to see some attention to the issues above. If you can't get to it we can either fix it after the fact or start a new PR.

@lonnieezell
Copy link
Member

I get the feeling @jason-napolitano is not going to be able to get this one. Will have to update myself manually. Not sure I have the time tonight to do that, but we'll see.

@lonnieezell lonnieezell merged commit 1abccce into codeigniter4:develop Aug 28, 2019
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.

3 participants