Re: [Pulp-dev] Pulp Code Owners
+1. My understanding is that this will not directly limit who can review or merge code, but should streamline the review process by notifying relevant parties? On Mon, Aug 13, 2018 at 5:29 PM, David Davis wrote: > We have come up with initial proposal of how to use code owners feature in > Pulp. Feedback on the initial proposal below is welcome. I will try to > collect the feedback and open a PUP by the end of the week. Thanks! > > > # Problem Statement > > For Pulp's review process, there are several areas we could improve: > > 1. It’s not always clear who should review files. Over time we have > developed subject matter experts for different areas of the codebase, but > these are not codified anywhere. It would be useful for us to define teams > need to review projects using code owners. > > 2. PRs go unnoticed. Github has a request-review feature, but only members > of the github organization can click 'request review' button. It would be > great if when a PR is opened people automatically received PR review > requests. > > > # Team Examples > > Functional Tests - The QE Team should be code owners of functional tests > that test core or core-maintained plugins > The Tasking System - bmbouter, daviddavis, and dalley are the SME in this > area > > > # Solution > > 1. Configure the code-owners feature of Github ( > https://blog.github.com/2017-07-06-introducing-code-owners/). It will > allow a team of 2 or more people to be notified and asked for review when a > PR modifies a file within a certain area of the code. > > 2. Require code-owner review to merge. This is described in this section: > https://blog.github.com/2017-07-06-introducing-code-owners/# > an-extra-layer-of-code-security > > > # Process > > The code owner role is not related to the commit bit. It's designed to get > better reviews. Well reviewed work can be confidently merged by anyone with > the commit bit. > > To make a change to code owners, open a PR with the changes, and call for > a lazy consensus vote by mailing list. Similar to the PUP decision making > process, voting must be open for 10 days, and the committers of the > respective repository are voting. > > The code owners file itself should be owned by the core committers of the > repository. > > > ___ > Pulp-dev mailing list > Pulp-dev@redhat.com > https://www.redhat.com/mailman/listinfo/pulp-dev > > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
[Pulp-dev] Pulp Code Owners
We have come up with initial proposal of how to use code owners feature in Pulp. Feedback on the initial proposal below is welcome. I will try to collect the feedback and open a PUP by the end of the week. Thanks! # Problem Statement For Pulp's review process, there are several areas we could improve: 1. It’s not always clear who should review files. Over time we have developed subject matter experts for different areas of the codebase, but these are not codified anywhere. It would be useful for us to define teams need to review projects using code owners. 2. PRs go unnoticed. Github has a request-review feature, but only members of the github organization can click 'request review' button. It would be great if when a PR is opened people automatically received PR review requests. # Team Examples Functional Tests - The QE Team should be code owners of functional tests that test core or core-maintained plugins The Tasking System - bmbouter, daviddavis, and dalley are the SME in this area # Solution 1. Configure the code-owners feature of Github ( https://blog.github.com/2017-07-06-introducing-code-owners/). It will allow a team of 2 or more people to be notified and asked for review when a PR modifies a file within a certain area of the code. 2. Require code-owner review to merge. This is described in this section: https://blog.github.com/2017-07-06-introducing-code-owners/#an-extra-layer-of-code-security # Process The code owner role is not related to the commit bit. It's designed to get better reviews. Well reviewed work can be confidently merged by anyone with the commit bit. To make a change to code owners, open a PR with the changes, and call for a lazy consensus vote by mailing list. Similar to the PUP decision making process, voting must be open for 10 days, and the committers of the respective repository are voting. The code owners file itself should be owned by the core committers of the repository. ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] Pulp smash test docs and issues
Thanks for the reply Kersom. I responded inline. On Mon, Aug 13, 2018 at 3:49 PM, Kersom wrote: > Thank you for this thread. > > Currently there are dedicated documents for Pulp-Smash on readthedocs. [0] > Issues related to Pulp-Smash itself should be filed here [1] > +1 > > Documentation for Pulp 2 tests were created on readthedocs [2] after these > tests were moved from Pulp-Smash. > Issues related to Pulp 2 should be filed here [3] > +1 > > Pulp 3 tests were migrated to Pulp repositories, but there are no > documentation for them right now. In my opinion, documentation for tests, > code standards for tests, and examples will drive more contributions. I am > not sure what the best option is to generate and host these docs. > For topics specific to a plugin, I agree it should go in the repo w/ the functional tests themselves. Some plugins host those docs via github.com browseable readme's, (e.g. https://github.com/pulp/pulp_ansible/#pulp-ansible) or via a read the docs website (e.g. http://pulp-python.readthedocs.io/en/3.0-dev/). The docs.pulpproject.org site can't host plugin docs for Pulp3, only core code. > > I think we can use the plugin issue tracker to track tests related to a > certain plugin. Perhaps a certain field, or anchor can be used to allow > filters for issues that require tests. Maybe the same approach can be used > for the pulp core as well. > To demo an idea, I just added a 'Functional Test' tag which should be available on all Redmine projects. If we want to rename it, or delete it, we can. How would that work? > [0] https://pulp-smash.readthedocs.io/en/latest/ > [1] https://github.com/PulpQE/pulp-smash > [2] https://pulp-2-tests.readthedocs.io/en/latest/ > [3] https://github.com/PulpQE/Pulp-2-Tests > > > > On Fri, Aug 10, 2018 at 10:16 AM, Brian Bouterse > wrote: > >> >> >> On Thu, Aug 9, 2018 at 5:42 PM, Daniel Alley wrote: >> >>> I think this is fine where it is. pulp2 is going into maintenance mode at some point here soon. >>> >>> >>> That makes sense for the Pulp 2 smash test docs, but it's still a >>> problem if we want to have smash test docs for Pulp 3 (which, we do) >>> >> That's true, we do need it to live somewhere. >> >> The main thing I want to avoid is trying to include content from one >> repo, i.e. PulpQE/pulp-smash to be published through the docs of the sphinx >> project in pulp/pulp. In terms of what pulp-smash offers and how to use it, >> I think that should be its own site, separate from Pulp's docs. >> Additionally, I could imagine a section in our docs either recommending >> pulp-smash and linking to the pulp-smash docs, and maybe expanding on its >> examples some. Is this kind of what you imagined? How is it >> similar/different? >> >> >>> On Thu, Aug 9, 2018 at 5:12 PM, Brian Bouterse >>> wrote: >>> Thanks for bringing this up. I put some responses inline. On Thu, Aug 9, 2018 at 4:22 PM, David Davis wrote: > One of the things that may not make sense anymore is how we document > and track issues for pulp-smash tests given that these tests no longer > reside in the pulp-smash repo. Currently, all the test-related issues are > tracked here[0]. > > With the tests no longer in the pulp-smash repo, I wonder if it makes > sense to maybe move them somewhere else like into redmine for Pulp 3 (or > the pulp-2-tests[1] repo for Pulp 2). > +1 to moving issues about the testing of a plugin to that plugin's tracker > The other question is about documentation. Currently the pulp-smash > test documentation is hosted on RTD (e.g. https://pulp-2-tests.rea > dthedocs.io/en/latest/). Should this documentation live alongside the > core/plugin docs? > I think this is fine where it is. pulp2 is going into maintenance mode at some point here soon. > QE would like to hear feedback as to how to proceed by August 20, 2018 > so please respond by then. > > Thanks. > > [0] https://github.com/pulpqe/pulp-smash/issues > [1] https://github.com/PulpQE/Pulp-2-Tests > > David > > ___ > Pulp-dev mailing list > Pulp-dev@redhat.com > https://www.redhat.com/mailman/listinfo/pulp-dev > > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev >>> >> >> ___ >> Pulp-dev mailing list >> Pulp-dev@redhat.com >> https://www.redhat.com/mailman/listinfo/pulp-dev >> >> > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] PUP-1 template missing field
+1 On Mon, Aug 13, 2018 at 3:57 PM, Daniel Alley wrote: > +1 > > On Mon, Aug 13, 2018 at 3:35 PM, David Davis > wrote: > >> I am making a small tweak to PUP-1 to include version in the template. I >> forgot to do so when I added the revision process to PUP-1. >> >> https://github.com/pulp/pups/pull/13 >> >> Please vote by August 25, 2018. Again the options are: >> >> +1: "Will benefit the project and should definitely be adopted." >> +0: "Might benefit the project and is acceptable." >> -0: "Might not be the right choice but is acceptable." >> -1: "I have serious reservations that need to be thought through and >> addressed." >> >> Thanks. >> >> David >> >> ___ >> Pulp-dev mailing list >> Pulp-dev@redhat.com >> https://www.redhat.com/mailman/listinfo/pulp-dev >> >> > > ___ > Pulp-dev mailing list > Pulp-dev@redhat.com > https://www.redhat.com/mailman/listinfo/pulp-dev > > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] PUP-1 template missing field
+1 On Mon, Aug 13, 2018 at 3:35 PM, David Davis wrote: > I am making a small tweak to PUP-1 to include version in the template. I > forgot to do so when I added the revision process to PUP-1. > > https://github.com/pulp/pups/pull/13 > > Please vote by August 25, 2018. Again the options are: > > +1: "Will benefit the project and should definitely be adopted." > +0: "Might benefit the project and is acceptable." > -0: "Might not be the right choice but is acceptable." > -1: "I have serious reservations that need to be thought through and > addressed." > > Thanks. > > David > > ___ > Pulp-dev mailing list > Pulp-dev@redhat.com > https://www.redhat.com/mailman/listinfo/pulp-dev > > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] Pulp smash test docs and issues
Thank you for this thread. Currently there are dedicated documents for Pulp-Smash on readthedocs. [0] Issues related to Pulp-Smash itself should be filed here [1] Documentation for Pulp 2 tests were created on readthedocs [2] after these tests were moved from Pulp-Smash. Issues related to Pulp 2 should be filed here [3] Pulp 3 tests were migrated to Pulp repositories, but there are no documentation for them right now. In my opinion, documentation for tests, code standards for tests, and examples will drive more contributions. I am not sure what the best option is to generate and host these docs. I think we can use the plugin issue tracker to track tests related to a certain plugin. Perhaps a certain field, or anchor can be used to allow filters for issues that require tests. Maybe the same approach can be used for the pulp core as well. [0] https://pulp-smash.readthedocs.io/en/latest/ [1] https://github.com/PulpQE/pulp-smash [2] https://pulp-2-tests.readthedocs.io/en/latest/ [3] https://github.com/PulpQE/Pulp-2-Tests On Fri, Aug 10, 2018 at 10:16 AM, Brian Bouterse wrote: > > > On Thu, Aug 9, 2018 at 5:42 PM, Daniel Alley wrote: > >> I think this is fine where it is. pulp2 is going into maintenance mode at >>> some point here soon. >> >> >> That makes sense for the Pulp 2 smash test docs, but it's still a problem >> if we want to have smash test docs for Pulp 3 (which, we do) >> > That's true, we do need it to live somewhere. > > The main thing I want to avoid is trying to include content from one repo, > i.e. PulpQE/pulp-smash to be published through the docs of the sphinx > project in pulp/pulp. In terms of what pulp-smash offers and how to use it, > I think that should be its own site, separate from Pulp's docs. > Additionally, I could imagine a section in our docs either recommending > pulp-smash and linking to the pulp-smash docs, and maybe expanding on its > examples some. Is this kind of what you imagined? How is it > similar/different? > > >> On Thu, Aug 9, 2018 at 5:12 PM, Brian Bouterse >> wrote: >> >>> Thanks for bringing this up. I put some responses inline. >>> >>> On Thu, Aug 9, 2018 at 4:22 PM, David Davis >>> wrote: >>> One of the things that may not make sense anymore is how we document and track issues for pulp-smash tests given that these tests no longer reside in the pulp-smash repo. Currently, all the test-related issues are tracked here[0]. With the tests no longer in the pulp-smash repo, I wonder if it makes sense to maybe move them somewhere else like into redmine for Pulp 3 (or the pulp-2-tests[1] repo for Pulp 2). >>> +1 to moving issues about the testing of a plugin to that plugin's >>> tracker >>> >>> The other question is about documentation. Currently the pulp-smash test documentation is hosted on RTD (e.g. https://pulp-2-tests.rea dthedocs.io/en/latest/). Should this documentation live alongside the core/plugin docs? >>> I think this is fine where it is. pulp2 is going into maintenance mode >>> at some point here soon. >>> >>> QE would like to hear feedback as to how to proceed by August 20, 2018 so please respond by then. Thanks. [0] https://github.com/pulpqe/pulp-smash/issues [1] https://github.com/PulpQE/Pulp-2-Tests David ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev >>> >>> ___ >>> Pulp-dev mailing list >>> Pulp-dev@redhat.com >>> https://www.redhat.com/mailman/listinfo/pulp-dev >>> >>> >> > > ___ > Pulp-dev mailing list > Pulp-dev@redhat.com > https://www.redhat.com/mailman/listinfo/pulp-dev > > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
[Pulp-dev] PUP-1 template missing field
I am making a small tweak to PUP-1 to include version in the template. I forgot to do so when I added the revision process to PUP-1. https://github.com/pulp/pups/pull/13 Please vote by August 25, 2018. Again the options are: +1: "Will benefit the project and should definitely be adopted." +0: "Might benefit the project and is acceptable." -0: "Might not be the right choice but is acceptable." -1: "I have serious reservations that need to be thought through and addressed." Thanks. David ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] 'id' versus 'pulp_id' on Content
On 08/07/2018 11:47 AM, Jeff Ortel wrote: After long consideration, I have had a change of heart about this. I think. In short, Pulp's data model has unique requirements that make it acceptable to deviate from common convention regarding ID as the PK. Mainly that the schema is extensible by plugin writers. Given the plugin architecture, I think it's reasonable to think of "core" fields like: ID, CREATED and LAST_MODIFIED as metadata. Although, the ID is harder to fit in this semantic, I think it's reasonable to do for consistency and to support the user query use-case re: content having an natural ID attribute. Taking this further, the /href/ attributes /could/ be though of in the same category. With this in mind, I'm thinking that the leading underscore (_) could be used broadly to denote /generated/ /or metadata/ fields and the following would be reasonable: _id _created _last_updated I'm convinced that all tables should have _created. Knowing when something is created helps fulfill many common use cases and is essential for troubleshooting. I am open to including _last_updated only on mutable entities . Depending on the number (ratio) of mutable/immutable entities, we could support this with either an additional Model class eg: MutableModel or just add _last_updated on concrete models. Either way, the column (attribute) needs to be named consistently. _href _added_href _removed_href _content_href I highly value consistency so this applies to the entire schema. This will introduce a few fairly odd things into the schema that I /suppose/ we can live with. - Two fields on /some/ tables named (ID , _ID). To mitigate confusion, we should serialize the *pk* and not*_id*. This will also be consistent with *pk* parameters passed in. - I expect django will generate foreign key fields with double understores. Eg: content__id I'm still -1 for using a /pulp_/ prefix. Thoughts? ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
