Skip to content

Add sonata-project/notification-bundle recipe #302

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

Merged
1 commit merged into from
May 17, 2018

Conversation

covex-nn
Copy link
Contributor

Q A
License MIT

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

@covex-nn
Copy link
Contributor Author

covex-nn commented Feb 24, 2018


~~~To use SonataNotificationBundle with `JMS Serialiser`, require `jms/serializer-bundle` and configure application in `config/packages/jms_serializer.yaml`:~~~

```yaml
jms_serializer:
    metadata:
        directories:
            app:
                namespace_prefix: "App"
                path: "%kernel.project_dir%/metadata/jms_serializer"
```

~~~Directory `metadata/doctrine/` contains `SonataNotificationMessage.orm,xml` file with xml doctrine orm mapping metadata.~~~

~~~To use SonataNotificationBundle with `Doctrine ORM`, require `doctrine/orm doctrine/doctrine-bundle`, and configure application in `config/packages/doctrine.yaml`:~~~

```yaml
doctrine:
    orm:
        mappings:
            SonataApp:
                is_bundle: false
                type: xml
                dir: '%kernel.project_dir%/metadata/doctrine'
                prefix: 'App\Entity'
                alias: SonataApp
```

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

@kunicmarko20
Copy link
Contributor

As we can't determine if the user is using Entities, PHPCR or Documents we can't go this way and we will have to fall back to using easyextends. So you can remove the Entity, serialization and orm configuration. Can you make an RFC on https://github.com/sonata-project/dev-kit and we can further discuss this there?

@covex-nn
Copy link
Contributor Author

@kunicmarko20 i thought, that removing ORM annotations will solve that problem =) i should ask first about it, sorry

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

@covex-nn covex-nn changed the title [WIP] Add sonata-project/notification-bundle recipe Add sonata-project/notification-bundle recipe Apr 20, 2018
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

@covex-nn
Copy link
Contributor Author

@kunicmarko20 PR is ready, please review

@kunicmarko20
Copy link
Contributor

Good job, now we need packs

@covex-nn
Copy link
Contributor Author

@symfony-flex-server review please

@covex-nn
Copy link
Contributor Author

covex-nn commented Apr 21, 2018

@Nyholm the README.rst says that after a trusted user approves PR, it will be automatically merged; and a The user is a member of the organizations (publicly visible) for all modified packages is a trusted user too! @kunicmarko20 is a member of @sonata-project, but a PR is still not merged yet. How does this magic works? =)

@covex-nn
Copy link
Contributor Author

covex-nn commented Apr 21, 2018

@symfony-flex-server merge please

@kunicmarko20
Copy link
Contributor

kunicmarko20 commented Apr 24, 2018

cc @jordisala1991 @greg0ire @core23 @OskarStark can someone approve this?

@kunicmarko20
Copy link
Contributor

@Nyholm how do we get this merged?

@covex-nn
Copy link
Contributor Author

Maybe someone, not a member of @sonata-project, have to approve PR?

@covex-nn
Copy link
Contributor Author

@symfony-flex-server review please

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

@jordisala1991
Copy link
Contributor

jordisala1991 commented Apr 24, 2018

@symfony-flex-server merge please

or not, if you don't want to

@kunicmarko20
Copy link
Contributor

Every other PR that was merged had to be approved by Symfony Core, so I guess we can't just merge it like this.

@covex-nn
Copy link
Contributor Author

covex-nn commented Apr 24, 2018

@kunicmarko20 @jordisala1991 i hoped, that a member of @sonata-project is a trusted user and this feature works. May be we should wait for a approval from other user? not me and not @sonata-project member and not Core Team member?

@jordisala1991
Copy link
Contributor

how do we become trusted users?

@jordisala1991
Copy link
Contributor

The user is a member of the organizations (publicly visible) for all modified packages.

Given this description, I should be a trusted user, why can't I merge this?

@covex-nn
Copy link
Contributor Author

covex-nn commented Apr 25, 2018

Can someone add a comment on the pull request that should have been merged automatically? I will have a look later on.

@fabpot you asked to add comment here, this PR was not merged automaticaly

@patrickbussmann
Copy link

Is there a reason why pull requests without errors like this will not be merged?

@greg0ire
Copy link
Contributor

@patrickbussmann I think a bug is the most probable reason, and it has yet to be determined, see #302 (comment)

@fabpot
Copy link
Member

fabpot commented May 17, 2018

@jordisala1991 you need to approve the pull request, which you have not done yet.

@OskarStark
Copy link
Contributor

@fabpot he did and me either and we are both members of sonata-project:
screenshot 2018-05-17 08 51 44

@covex-nn
Copy link
Contributor Author

@symfony-flex-server review please =)

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

@fabpot
Copy link
Member

fabpot commented May 17, 2018

I will have a look :)

@OskarStark
Copy link
Contributor

Thank you very much!

@ghost ghost merged commit 5253d9e into symfony:master May 17, 2018
ghost pushed a commit that referenced this pull request May 17, 2018
@fabpot
Copy link
Member

fabpot commented May 17, 2018

And fixed :)

@covex-nn
Copy link
Contributor Author

Hooray =) Thank you!

@covex-nn covex-nn deleted the sonata-notification branch May 17, 2018 08:46
This pull request was closed.
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.

7 participants