Skip to content
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

Conway Shim API #789

Merged
merged 32 commits into from Oct 10, 2023
Merged

Conway Shim API #789

merged 32 commits into from Oct 10, 2023

Conversation

nickcastel50
Copy link
Contributor

This is a full implementation of a shim for the Web-IFC read API capabilities. I will place build instructions here since I don't want to edit the developer guide just yet until it lands, and there are currently no extra build instructions in the readme.

Build Instructions:

  1. yarn add ./bldrs-conway-v0.0.1.tgz --production
  2. yarn add shx

Conway Shim:

  1. yarn build-conway
  2. yarn serve-share-conway

Original Web-IFC

  1. yarn build-webifc
  2. yarn serve-share-webifc

@netlify
Copy link

netlify bot commented Sep 14, 2023

Deploy Preview for bldrs-share ready!

Name Link
🔨 Latest commit 46fad93
🔍 Latest deploy log https://app.netlify.com/sites/bldrs-share/deploys/6525a78d1142c00008e87a94
😎 Deploy Preview https://deploy-preview-789--bldrs-share.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 25 (🔴 down 1 from production)
Accessibility: 84 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@pablo-mayrgundter
Copy link
Member

Per convo, logging and error should accumulate to a vector during Init, OpenModel and Geometry/Mesh calls, etc. to optionally be accessed after each call by a method compatible with web-ifc, GetAndClearErrors.

Only variant we thought up was to keep a map by IFC type, or perhaps by error type, to keep counts instead of 1:1 vector, since there is usually a high count involved that is slow to log.

@ConorStokes
Copy link

ConorStokes commented Sep 19, 2023 via email

@nickcastel50 nickcastel50 marked this pull request as ready for review September 26, 2023 16:13
@pablo-mayrgundter
Copy link
Member

Ok, builds for me.. nice :)

Please respond to My&Conor's comments about logging.

Couples questions on build. Where did you land with Ogali's build process? Should we see 2 previews here? Also, is this safe to check-in and won't affect production?

@nickcastel50
Copy link
Contributor Author

Ok, builds for me.. nice :)

Please respond to My&Conor's comments about logging.

Couples questions on build. Where did you land with Ogali's build process? Should we see 2 previews here? Also, is this safe to check-in and won't affect production?

Regarding logging, a logging proxy that maps to multiple targets makes sense. I think this should come in a later PR though as geometry type coverage is taking precedence, and then we can circle back and hit logging across Conway / Conway-Geom, Shim, Sentry, etc.

As for build process, it looks like it's just generating one link still. If you checked this in, it would currently deploy Conway Shim and not Web-IFC, which might not be what we want yet due to coverage. We should page @oogali about how we can deploy this using the subdomain(s) as discussed.

@pablo-mayrgundter
Copy link
Member

Ok yep, should check in without overriding prod.. LGTM and I'll hand off to Ogali now

@pablo-mayrgundter
Copy link
Member

image

@netlify
Copy link

netlify bot commented Sep 29, 2023

Deploy Preview for bldrs-share-v2 ready!

Name Link
🔨 Latest commit 46fad93
🔍 Latest deploy log https://app.netlify.com/sites/bldrs-share-v2/deploys/6525a78d2af3dc0008bba895
😎 Deploy Preview https://deploy-preview-789--bldrs-share-v2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pablo-mayrgundter
Copy link
Member

Hey Nick,

We brought up at mtg the possibility that all you need to do here is flip the default from conway to web-ifc. Is that all that's left to determine?

If so, I suggest trying that and submitting, but stand by to revert if need be.

How's the sound?

@nickcastel50
Copy link
Contributor Author

Hey Nick,

We brought up at mtg the possibility that all you need to do here is flip the default from conway to web-ifc. Is that all that's left to determine?

If so, I suggest trying that and submitting, but stand by to revert if need be.

How's the sound?

Hey @pablo-mayrgundter, Ogali and I took a look on Friday. In addition to changing the default "build" back to web-ifc (that change is already here), there are a few changes I just pushed up for Github actions, so it will actually test and run both the shim and original web-ifc backend. Once I can confirm the Github Action changes work we should be good to go.

package.json Show resolved Hide resolved
@nickcastel50 nickcastel50 merged commit d31b5e3 into bldrs-ai:main Oct 10, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants