Pnpjs: [2.0.0] sub module: SP-items

Created on 7 May 2019  路  11Comments  路  Source: pnp/pnpjs

This issue tracks the review and preparation of a sub module for the 2.0.0 project. To finalize a sub-module the follow steps should be performed:

  • [ ] Code review for TODO (do), commented out code (remove), ensure interfaces are prefixed with an "I", or not using await
  • [ ] Ensure every property/method has at least one test
  • [ ] Ensure all of the interface method/properties are commented
  • [ ] Remove comments from class implementation files
  • [ ] Ensure the docs page is updated to mention each method/property with at least a minimal example
  • [ ] Search in closed issue by the label "area: sample" to see if any apply to the module you are review and add those as appropriate to the documentation
  • [ ] Review/compare the code in the 1.x branch to ensure any fixes, updates, or changes are in 2.0

Once complete submit one PR per module for final review. Please make the title match the issue title and reference the issue in the body of the PR. It will then be reviewed so please check back for any feedback or questions.

Most helpful comment

I would like to work on and complete the tasks for this sub-module
(although I might need a little help getting started!)

@juliemturner @PopWarner @simonagren

All 11 comments

I would like to work on and complete the tasks for this sub-module
(although I might need a little help getting started!)

@juliemturner @PopWarner @simonagren

Hi @MartinHatch. Great to have you.
Would you like to have a look at it first and then formulate questions?

@simonagren I think I'm ok for now ...

Assuming this item is fairly exclusively ..

  • pnpjs/packages/sp/

    • ../src/items.ts

    • ../docs/items.md

    • ../tests/items.tests.ts

Is there anywhere else I should need/expect to update?

@simonagren ok two general questions on convention ...

  1. Are we definitely using an I prefix for interfaces (e.g. class Foo implements IFoo) .. given the TypeScript documentation specifically recommends that you don't do this?
    https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines
    _("Do not use "I" as a prefix for interface names.")_

  2. Do you want a "double I" for interfaces for objects which already start with "I"
    For example:

  • IItemAddResult
  • IItemUpdateResult

@simonagren sorry another question.

in items.test.ts > Almost every call to the "toUrl()" is complaining that the method does not exist.

I can see the public function declared in "queryable.ts" .. should this automatically "bubble up"? I'm wondering if there is a missing import or something in the test file.

@MartinHatch I'm sorry. I'm back from the dead now. I needed a break from everything :) I will get to your questions in the beginning of this week.

Hi @MartinHatch.....hope all is going well with ya!

Wanted to see if you still had that open question or if you were able to proceed?

We are aiming for a Release Candidate Beta by the end of September and wanted to follow up with the issues which are "In progress" to see how things are going.

Thank you so much again for helping out and looking forward to hearing from you. :)

Still have this open question .. nobody has gotten back to me and TBH it fell off my radar

My apologies @MartinHatch, let me try to track it down for you. 馃憤

Hey @MartinHatch - still interested in working this one?

For the toUrl method sometimes vs code get confused. If you close vs code, run npm run build, and reopen vs code everything should be sorted. Or if you just hit F5 things should work correctly.

I am going to go ahead and take this one over so I can get the tests written and close it out. Thanks!

Was this page helpful?
0 / 5 - 0 ratings