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

fix: declared types in models repository - I153 #173

Closed

Conversation

apoorvsxna
Copy link

Closes #153

Fixed incorrectly generated import prefix on the models website.

Changes

  • Added statement to declare the SERVER_ROOT environment variable.
  • Corrected syntax of generated import statement.

Screenshots or Video

  • Current state-

Screenshot (93)

  • After fix-

Screenshot (94)

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Commits messages follow AP format
  • Merging to main from fork:branchname

build.js Outdated
@@ -14,6 +14,7 @@

'use strict';

process.env.SERVER_ROOT = "https://models.accordproject.org";
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of hardcoding this in build.js can you please set it in package.json where the script is called?

Copy link
Author

Choose a reason for hiding this comment

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

Made some changes. SERVER_ROOT is now declared as an environment variable at the time of execution.

@@ -38,7 +38,7 @@
<div class="content box">
<pre style="border: 0; background-color: transparent;">
<code class="js">
{% for decl in modelFile.getAllDeclarations() %}import {{decl.getFullyQualifiedName()}} from {{serverRoot}}{{filePath}}.cto
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the import should be quoted in CTO syntax.

Copy link
Author

@apoorvsxna apoorvsxna Mar 28, 2024

Choose a reason for hiding this comment

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

If quotes are removed, the part of the statement after "https:" is displayed as a comment, due to syntax highlighting (using highlight.js). It probably detects the code as javascript

Screenshot (99)

As a workaround, I ended up skipping syntax-highlighting for this code block altogether. So this is how it looks now-

Screenshot (98)

@apoorvsxna
Copy link
Author

@dselman. Could you please see if any changes are required?

@dselman dselman closed this Nov 7, 2024
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.

Import prefix is missing
2 participants