Skip to content

docs: improve migration namespace descriptions#5915

Merged
kenjis merged 14 commits intocodeigniter4:developfrom
kenjis:fix-docs-migration-namespace
Apr 22, 2022
Merged

docs: improve migration namespace descriptions#5915
kenjis merged 14 commits intocodeigniter4:developfrom
kenjis:fix-docs-migration-namespace

Conversation

@kenjis
Copy link
Copy Markdown
Member

@kenjis kenjis commented Apr 21, 2022

Description
Setting null in $namespace means all namespaces. It is not intuitive.
So we better explain it in all relevant places.

  • add explanation for null in $namespace
  • improve testing/database.rst format
  • improve dbmgmt/migration.rst format

Checklist:

  • Securely signed commits
  • [] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [] Unit testing, with >80% coverage
  • User guide updated
  • [] Conforms to style guide

@kenjis kenjis added the documentation Pull requests for documentation only label Apr 21, 2022
@lonnieezell
Copy link
Copy Markdown
Member

I think this looks good but would it be easy enough to make it do all namespaces when either null or --all were present? I think that might make it more intuitive while not harming BC.

@kenjis
Copy link
Copy Markdown
Member Author

kenjis commented Apr 21, 2022

@lonnieezell Sorry, I can't get what you mean.
null is a property value, but --all is a command option.

@lonnieezell
Copy link
Copy Markdown
Member

Sorry. I meant it would be nice to set $namespace to either null or the string --all within a test case to have it run all migrations. That way the property value and the command option are the same. Or maybe all would be enough?

class MyTestCase extends DatabaseTestCase
{
    protected $namespace = '--all';
    // or
    protected $namespace = 'all';
}

Copy link
Copy Markdown
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.

Very nice explanations.

@kenjis kenjis merged commit 73c4e25 into codeigniter4:develop Apr 22, 2022
@kenjis kenjis deleted the fix-docs-migration-namespace branch April 22, 2022 12:22
@kenjis
Copy link
Copy Markdown
Member Author

kenjis commented Apr 22, 2022

@MGatner How about adding new special strings for all namespaces as @lonnieezell says?

protected $namespace = 'all';
// or
protected $namespace = '*';

Personally $namespace = '--all' seems odd to me.

@MGatner
Copy link
Copy Markdown
Member

MGatner commented Apr 23, 2022

I'm not opposed. Out of all those I prefer * since there is no possible confusion that it is a namespace.

Honestly I don't know how to feel about this feature. IMO running tests against a "partially-migrated" database seems like setup for a logical failure. I frequently have migrations modifying the same table across namespaces (like Myth:Auth creates users then my app adds users.firstname) and these would either fail or be a false representation of the table if I only ran one namespace or the other.

What selective namespace migrating does add is efficiency, but I think this is a problem better solved with other tools (see my other comment about generating a "one-and-done" schema, like Rails).

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

Labels

documentation Pull requests for documentation only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants