Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.57%. Comparing base (a9cc3af) to head (ded7b33).
Report is 2 commits behind head on main.

@@           Coverage Diff           @@
##             main     #854   +/-   ##
=======================================
  Coverage   88.57%   88.57%           
=======================================
  Files         129      129           
  Lines        1725     1725           
  Branches      213      220    +7     
=======================================
  Hits         1528     1528           
+ Misses        197      182   -15     
- Partials        0       15   +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I ran the sdk generator locally and it doesn't look like it's up to snuff. For instance:

ApplicationForm.ts (main branch)

import {
	ApplicationFormField,
} from './ApplicationFormField';

import { Writable } from './Writable';

export interface ApplicationForm {
	readonly id: number;
	opportunityId: number;
	readonly version: number;
	fields?: Array<ApplicationFormField>;
	readonly createdAt: Date;
}

export type WritableApplicationForm = Writable<ApplicationForm>

ApplicationForm.ts (generated from this branch)

import { Writable } from './Writable';

export interface ApplicationForm {
	readonly id: any;
	opportunityId: any;
	readonly version: any;
	fields?: any;
	readonly createdAt: any;
}

export type WritableApplicationForm = Writable<ApplicationForm>

I'll look into whether or not this is a swagger-codegen limitation or if I'm doing something wrong here.

Based on swagger-api/swagger-codegen#12210 it looks like codegen doesn't support 3.1.0 yet -- it isn't clear if / when it ever will, though it does seem that the core technologies have been updated to support it, so maybe there's just a dependency version that needs updating somewhere.

Once this PR is merged we should be able to finish this one: PhilanthropyDataCommons/sdk#26

Once this PR is merged we should be able to finish this one: PhilanthropyDataCommons/sdk#26

You mean the opposite, right?

From the PR description:

It also makes a small change to remove nullability from a few fields, which should no longer be nullable. This was done as part of the migration because it appears v3.1 changes the way the nullable keyword works.

Is this outdated? I don't see anything referencing nullability.

@reefdog whoops I accidentally nuked the nullable change, I'll either confirm it's needed and add that back in, or will update the PR description. If I add the changes I may ping you for another review, if I just edit the PR description I will not!

That said, we do want to merge the SDK generator changes first since this PR is what will trigger the build / publication of the sdk (and if we merge this before updating the generator, the generator will create a very broken SDK).

EDIT: I'm now seeing the problem -- I wrote "once this pr is merged we'll be able to finish this one" and there is absolutely no way of knowing which "this" refers to THIS pr vs the linked PR. Good job Dan.

@slifty 😆 Yes, that was the source of my confusion. Pronoun antecedents!

We paused the project around May 25th so this got stale!

Reviewing everything and will nudge this to the next step so we can benefit from 3.1 soon