-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
restructure repo to publish types properly to npm registry #135
Comments
@shravanhariharan2 gonna tackle this next since Sage Team and Dimensions are going to be using our API. Thoughts? Our goals:
I looked into some existing tools but they don't play well with lerna and weirdly rely on commit message structures which seem pretty liable to break since it's hard to enforce that on PR titles. But the general idea for them's that you commit to For publishing:
Long-term it'd be nice to automate versioning with a GitHub bot or something (it comments on your PR with a checklist like ["improvement";"patch";...], you check a box, and it commits your version bump), and auto-generate docs from code like TypeDoc but for Postman, but those aren't important right now. |
Couple comments/questions:
Are two commits necessary? And how are they supposed to be formatted (and is that taken care of by the CI before merge)?
Thoughts on also adding seeds/fakes as a package? Not sure how this would work since a lot of it requires implementation functionality, but a developer could work directly with portal state programatically. Not sure how useful this might be but let me know your thoughts.
Not sure what you mean by manual approval for check - does this appear in CircleCI as a task with no actual job to run? Also, this documentation refers to our Postman docs right? Right now that's only accessible from our ACM development account, [email protected]. I temporarily moved it there since free Postman only allows 25 shared requests (it prevented me from adding/editing any requests after that), so should we move to a paid plan ($12-15 per user per month)? Alternatively we could have everyone working on one Postman account to make it free free but thats a huge hassle to manage credentials Also, how does it work of two people have different PR's that both need to bump versions? You mentioned that we can update versions within the PR, but this might cause conflict with other PR's right? Or is that bump only on merging, where the other PR can merge those changes & bumps and bump their own after that? But then wouldn't they be blocked by the other PR merging first before they version bump? Overall, it seems pretty clean but a little complicated of a change just to add versioning for types/specs, but looks like this will help down the road w/ documentation + API wrappers and it doesn't look like there's any cleaner & automated alternatives. |
|
@shravanhariharan2 any other thoughts on this? |
Rest sounds good to me |
PR titles don't need to be specifically formatted, but I'll switch to uppercase imperative to match your PRs since that's more common. |
ok, and are we trying to match commit formats too? Like imperative vs. past, upper vs. lower, or does that not matter since the PR title is what gets commited to master anyway? |
Commits in PR branches don't matter. |
@sumeet-bansal @michl1001 there's been some interest in getting this set up within the next few weeks for ease of development in frontend for merch store (plus other QoL things for frontend with the API wrapper). Since we aren't necessarily working on any features currently, I'm looking to dig deeper into what we want out of a restructure like this. Some questions to consider / discuss:
initially might work but I'm not sure what's the best way to split implementation from spec in a package sense. I think we should also definitely consider writing the API wrapper now as well since it makes testing a lot easier. As I've been working more on testing I'm realizing how much easier it'd be to have actual API calls rather than directly using the controller methods, since things like validating |
I think our first goal's to publish types, and the API wrapper can be done afterwards. What can't we test with our current setup? |
One thing I'm unable to test now is if an error is thrown if a file passed in is of too large of a size to the upload controllers (e.g. event cover upload). From what it looks like a lot of the processing/validation happens under the hood of If that's a minor thing (which is what it seems like) then I can just continue writing some more tests to cover the essential things (events and attendance) before moving forward. |
Published types should only have required dependencies and we should limit the manual work required to version properly.
I imagine this would take the form of a multi-package repo managed by lerna or some similar tool, and possibly some semantic release tool like semantic-release kicked off by our CI pipeline.
The text was updated successfully, but these errors were encountered: