Pnpjs: [2.0.0] sub module: SP-files

Created on 7 May 2019  路  14Comments  路  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.

All 14 comments

@PopWarner / @patrick-rodgers Morning guys,
i can help with this one.

Hi @ValerasNarbutas - great, I have a huge set of changes I plan to merge later today, so you'll want to pull latest after I do. You can track #888 to see when it is merged.

Thanks @patrick-rodgers , i can wait.
just for the interest does tour changes also include debug/launch/main.ts file? some commended out code for deletion test sub-sites does not really work.

Thanks for the help @ValerasNarbutas! You rock! :)

LOL, ok. Can you be any more specific? I can add that to my list of 400 other things I need to get to. Or if you see an opportunity to improve/fix something please do.

i might just update it, and my mistake its test/main.ts commented cleanUpAllSubsites() function.
i get error like:
1) "before all" hook in "{root}":
TypeError: global.fetch is not a function
but most likely its just order where function cleanUpAllSubsites() is called.

Thanks for the help @ValerasNarbutas! You rock! :)

no, you guys rock :)

hey @patrick-rodgers ,
i have checked #888 looks great but before merging this could you first review pull request #894 ?
it contains minor fix in test main.js and some documentation for site users. I do see you have changed a lot man, looking forward for the new version :)
Thanks,

@ValerasNarbutas - would likely need you to pull latest and merge your work in after I merge 888. Thanks.

ok, @patrick-rodgers .
No worries than.

Hello, wanted to check on the status on this item as it is on the list of required things to be done before v2 can release. Please let us know an update - and if you won't have time to work on it, no worries just focused on getting things finalized. Thanks!

Hey Patrick,
i did not had time but i do this week and might be next week,
in short - on it :)

Last call - if you can't commit to having this done by the end of this week I need to take it over. No worries, just this is the last sub-module pending and needs to be finished. If you haven't had a chance to start cool - I can take it over. If you have done some work but aren't done, please submit a PR with your work to date so we can include it. Again, not upset/no hard feelings, just need to close this out. Thanks!

Hey @patrick-rodgers , go ahead and take over,
I have some test but they are very minor. I ,wont be able to finish this until the end of the week.

Was this page helpful?
0 / 5 - 0 ratings