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

Feature/Add Sage docs #3

Merged
merged 12 commits into from
Dec 21, 2020
Merged

Conversation

jacobcsmith
Copy link
Member

what

  • Adds the current draft of Sage documentation
  • Updates docusaurus config to allow for environment specific baseUrl

why

  • To migrate the existing internal documentation repo to Github

@jacobcsmith jacobcsmith requested a review from a team as a code owner December 18, 2020 17:57
@jacobcsmith
Copy link
Member Author

Errors found in custom.css seem to be an issue with CSSLint

@RothAndrew
Copy link
Contributor

Errors found in custom.css seem to be an issue with CSSLint

Correct. I unchecked "Prohibit Syntax Errors" in Codacy due to this issue. It shouldn't throw errors for the "expected RBRACE" issues again. Please push a new commit to retrigger the Codacy check

@jacobcsmith
Copy link
Member Author

Can we also ignore the console.log errors? The file in question is a utility run via the command line and not included in the built website

@RothAndrew
Copy link
Contributor

Can we also ignore the console.log errors? The file in question is a utility run via the command line and not included in the built website

The whole rule shouldn't be disabled, since it is still very valid for the Docusaurus stuff. The rule comes from ESLint, and you can configure ESLint using methods found here: https://eslint.org/docs/user-guide/configuring

I recommend either creating an .eslintignore file and adding the offending file to it, or an .eslintrc.json if you want to have more refined configuration than just ignoring the whole file

I'm curious though, why is this postman-to-md project being vendored like this rather than using it from its source (wherever that may be)?

@jacobcsmith
Copy link
Member Author

Is there a particular eslint config file we use which would match the rules used in Codacy?

We vendored postman-to-md because we needed to tailor the output to fit our needs and the library didn't have a way to customize the output outside of modifying the functions producing the output.

@RothAndrew
Copy link
Contributor

Is there a particular eslint config file we use which would match the rules used in Codacy?

We vendored postman-to-md because we needed to tailor the output to fit our needs and the library didn't have a way to customize the output outside of modifying the functions producing the output.

Okay. My recommendation would be to fork it to your personal github and make the changes there, then use it like you would use any other NPM tool. We can discuss later on whether it should be added as an open source project in this org.

Doing it this way should solve a lot of these issues since you are trying to have a node app and a web app in the same project.

At the very least, it needs to come out of the webapp folder and go into its own folder. Golang uses a folder called vendor which might work here (so instead of /webapp/src/postman-to-md you'd do /vendor/postman-to-md, though even then the projects vendored aren't being changed, just cloned.

If you are okay with moving it out to a different repo, then this should be resolved. If not, then we'll need to come up with a .eslintrc that works for this project (or maybe 2, one for the webapp and one for the vendored tool)

@RothAndrew
Copy link
Contributor

Also don't forget to comment /test, that's the trigger to get the Public GitHub/full-ci pipeline to run. You need to set your org visibility to public for Codefresh to be able to run it though

@jacobcsmith
Copy link
Member Author

Ok I'll move the postman to md code to a new repo. I've removed it from this repo for now to allow this PR to pass the checks.

@jacobcsmith
Copy link
Member Author

/test

1 similar comment
@jacobcsmith
Copy link
Member Author

/test

@jacobcsmith
Copy link
Member Author

/test

@JeniferBrown JeniferBrown merged commit 51b7f06 into saic-oss:main Dec 21, 2020
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.

3 participants