Skip to content

Ft page iterator #13#27

Merged
baywet merged 31 commits intomainfrom
ft-page-iterator-#13
Jan 27, 2022
Merged

Ft page iterator #13#27
baywet merged 31 commits intomainfrom
ft-page-iterator-#13

Conversation

@jobala
Copy link
Contributor

@jobala jobala commented Jan 17, 2022 •
edited
Loading

Overview

Adds PageIterator

Demo

Create a page iterator task

pageIterator, err := msgraphgocore.NewPageIterator(graphResponse, *reqAdapter, ParsableCons)
pageIterator.SetHeaders(map[string]string{"Content-Type": "application/json"})

Iterate

err := pageIterator.Iterate(func(pageItem interface{}) bool {
      item := pageItem.(graph.User)
      res = append(res, *item.GetId())
      return true
})

Pause & Resume

pageIterator.Iterate(func(pageItem interface{}) bool {
      item := pageItem.(graph.User)
      res = append(res, *item.GetId())
      
      return *item.GetId() != "2" // pauses when id == 2
})

// resumes iteration from user with id 3
pageIterator.Iterate(func(pageItem interface{}) bool {
      item := pageItem.(graph.User)
      res2 = append(res2, *item.GetId())
      
      return true
})

Notes

  • I made the Next & HasNext methods private because we have not reached a consensus on making them part of the Iterator's API

Testing

  • Run go test

Closes #13

baywet
baywet reviewed Jan 17, 2022
View reviewed changes
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

good first draft, added a few comments.

baywet
baywet suggested changes Jan 19, 2022
View reviewed changes
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

almost there! some minor changes. Don't hesitate to switch the PR to ready whenever it's ready.

@baywet
Copy link
Member

baywet commented Jan 21, 2022 •
edited
Loading

also CC @jasonjoh for the public docs update of this page

@jobala can you update the snippets in your initial comment with the latest changes so Jason has visibility please?

@jobala jobala marked this pull request as ready for review January 22, 2022 13:24
baywet
baywet suggested changes Jan 24, 2022
View reviewed changes
@baywet baywet mentioned this pull request Jan 24, 2022
Open
baywet
baywet suggested changes Jan 26, 2022
View reviewed changes
baywet
baywet suggested changes Jan 27, 2022
View reviewed changes
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

thanks for making the changes! only two docs comments missing and we should be good to go.
I also configured sonarcloud to exclude internal package from the metrics so it reflects the actual coverage.

@baywet baywet added this to the Core specification alignment milestone Jan 27, 2022
@baywet
Copy link
Member

baywet commented Jan 27, 2022

Also please update the changelog https://github.com/microsoftgraph/msgraph-sdk-go-core/blob/main/CHANGELOG.md

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 27, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

82.6% 82.6% Coverage
0.0% 0.0% Duplication

baywet
baywet approved these changes Jan 27, 2022
View reviewed changes
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

thanks for the excellent work!

@baywet baywet merged commit ec4b64e into main Jan 27, 2022
@baywet baywet deleted the ft-page-iterator-#13 branch January 27, 2022 14:17
@jobala jobala self-assigned this Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MIchaelMainer MIchaelMainer Awaiting requested review from MIchaelMainer

@nikithauc nikithauc Awaiting requested review from nikithauc

@ddyett ddyett Awaiting requested review from ddyett

@andrueastman andrueastman Awaiting requested review from andrueastman

@zengin zengin Awaiting requested review from zengin

@darrelmiller darrelmiller Awaiting requested review from darrelmiller

@peombwa peombwa Awaiting requested review from peombwa

@ramsessanchez ramsessanchez Awaiting requested review from ramsessanchez

1 more reviewer

@baywet baywet baywet approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jobala jobala

Labels

None yet

Projects

None yet

Milestone

Core specification alignment

Development

Successfully merging this pull request may close these issues.

Add page iterator

2 participants

@jobala @baywet