Skip to content

batches: query max unlicensed changesets count #929

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.

Sign up for GitHub

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

Jump to bottom
Merged
adeola-ak merged 15 commits into from
Jan 29, 2023
Merged

batches: query max unlicensed changesets count #929

adeola-ak merged 15 commits into from
Jan 29, 2023

Conversation

@adeola-ak
Copy link
Contributor

@adeola-ak adeola-ak commented Jan 26, 2023 •
edited
Loading

Closes #913

Change the 10 changeset limit for batch changes for unlicensed/free tier Sourcegraph instances from being hardcoded, to querying maxUnlicensedChangeset in the mutation where we create the batch spec so that error codesErrBatchChangesUnlicensed and ErrBatchChangesOverLimit returns value

CleanShot 2023-01-26 at 19 46 52@2x

Test plan

Test creating a batch change with more changesets than the current maxUnlicensedChangesets values as an unlicensed user

@adeola-ak adeola-ak requested review from a team and removed request for a team January 26, 2023 05:59
@courier-new courier-new requested a review from a team January 26, 2023 08:20
courier-new
courier-new reviewed Jan 26, 2023
View reviewed changes
Copy link
Contributor

@courier-new courier-new left a comment

Choose a reason for hiding this comment

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

Approach looks really solid to me! Sorry my review is basically a million nits to improve code quality a bit, but hopefully they make it easier for you to update things. 🙂

Would you mind... (I bet you can guess what I'm about to ask! 😜) ...sharing a screenshot of the new error, too?

@adeola-ak
Copy link
Contributor Author

adeola-ak commented Jan 26, 2023 •
edited
Loading

Approach looks really solid to me! Sorry my review is basically a million nits to improve code quality a bit, but hopefully they make it easier for you to update things. 🙂

Would you mind... (I bet you can guess what I'm about to ask! 😜) ...sharing a screenshot of the new error, too?

@courier-new i asked for a review and then i removed it when i realized i had some mistakes but thank you for your prompt and helpful review! it was like 1am and i was so tired that i couldnt find where to switch it to draft so i removed review instead lol and by the way i love the 'nitpicky' for code quality. Thank you!

adeola-ak and others added 4 commits January 26, 2023 09:38
@adeola-ak @courier-new
Update internal/batches/service/service.go
6577299 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@adeola-ak @courier-new
Update internal/batches/service/service.go
925c26f 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@adeola-ak
fix variables
5bd17cf
@adeola-ak
update verbs
4dd1aae
@adeola-ak adeola-ak requested a review from a team January 27, 2023 00:48
BolajiOlajide
BolajiOlajide reviewed Jan 27, 2023
View reviewed changes
adeola-ak and others added 2 commits January 27, 2023 09:08
@adeola-ak @BolajiOlajide
Update internal/batches/service/service.go
c53411d 
Co-authored-by: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
@adeola-ak @BolajiOlajide
Update internal/batches/ui/json_lines.go
2d6ea98 
Co-authored-by: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
@adeola-ak adeola-ak changed the title batches: query max ulicensed changesets count batches: query max unlicensed changesets count Jan 27, 2023
courier-new
courier-new approved these changes Jan 28, 2023
View reviewed changes
Copy link
Contributor

@courier-new courier-new left a comment

Choose a reason for hiding this comment

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

Just made a couple suggestions to preserve inline function documentation, otherwise 👍

adeola-ak and others added 5 commits January 29, 2023 02:29
@adeola-ak
Merge branch 'main' into aa/unlicensed
17cddcf
@adeola-ak @courier-new
Update internal/batches/service/service.go
839e001 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@adeola-ak @courier-new
Update internal/batches/service/service.go
3700c63 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@adeola-ak @courier-new
Update internal/batches/service/service.go
a23ac3f 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@adeola-ak
rename DetermineFeatureFlagsAndLicense
a601a9b
@adeola-ak adeola-ak enabled auto-merge (squash) January 29, 2023 07:43
@adeola-ak adeola-ak merged commit 715ed43 into main Jan 29, 2023
@adeola-ak adeola-ak deleted the aa/unlicensed branch January 29, 2023 07:44
scjohns pushed a commit that referenced this pull request Apr 24, 2023
@adeola-ak
batches: query max unlicensed changesets count (#929)
106a7d8 
replace hardcoded permitted unlicensed changesets count by querying maxUnlicensedChangesets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BolajiOlajide BolajiOlajide BolajiOlajide approved these changes

+1 more reviewer

@courier-new courier-new courier-new approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

batches: remove hardcoded max unlicensed changesets from src-cli TUI output

4 participants

@adeola-ak @courier-new @BolajiOlajide