-
Notifications
You must be signed in to change notification settings - Fork 128
docs: bring back vale rules #1930
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
base: master
Are you sure you want to change the base?
Conversation
rework Capitalization rules turn it back on
turn back Apify-specific rules fix all errors within sources/platform
Preview for this PR was built for commit |
Preview for this PR was built for commit |
Preview for this PR was built for commit |
Preview for this PR was built for commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on September 24
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Comment @cursor review
or bugbot run
to trigger another review on this PR
- [Set up environment variables in Apify Console](#set-up-environment-variables-in-apify-console) | ||
- [Access environment variables](#access-environment-variables) | ||
- [Use the `Configuration` class](#use-the-configuration-class) | ||
- [Build-time environment variables](#build-time-environment-variables) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, something (perhaps an extension to Cursor autogenerated full ToC, I'll fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found the culprit and pushing fix for this
Preview for this PR was built for commit |
Preview for this PR was built for commit |
.github/styles/Apify/Apify.yml
Outdated
# Product name changes | ||
'apify dashboard': Apify Console | ||
'apify console': Apify Console | ||
'Apify console': Apify Console |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's ignorecase: true
, is there a reason to have both cases mentioned?
Also, similar to the apify proxy
below, we could have the apify console
and the apify store
swapped for just Apify Console/Store.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that's true, I was trying to be a little extra, will remove i, and add your suggestion
This is the default option when your Actor's source code is hosted on the Apify platform. It offers quick previews and updates to your source code, easy file and directory browsing, and direct testing of the [`INPUT_SCHEMA.json`](/platform/actors/development/actor-definition/input-schema) on the Apify platform. | ||
|
||
A `Dockerfile` is mandatory for all Actors. When using the default NodeJS Dockerfile, you'll typically need `main.js` for your source code and `package.json` for [NPM](https://www.npmjs.com/) package configurations. | ||
A `Dockerfile` is mandatory for all Actors. When using the default NodeJS Dockerfile, you'll typically need `main.js` for your source code and `package.json` for [npm](https://www.npmjs.com/) package configurations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to add NPM
to npm
also to the swap rules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to try one more package once I deal with those rules and another PR with fixes to almost everything except H1's in Academy called Openly. I think it deals with a lot of those tech terms, might even lead to simplifying accept.txt
Let's explore the Actor structure. | ||
<!-- vale Apify.Capitalization = NO --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine there are multiple occurrences (or multiple occurrences can happen) of the .actors
folder. Would it make sense to define the swap rule as:
actor
/ actors
→ Actor
/Actors
.actor
→ .actor
I.e., with the leading space. Because I cannot see a situation where Actor would be used without a space before itself.
But it's true that it doesn't solve actor.json
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically Vale should ignore prose in inline code done with backticks and for the 95% it does... until it doesn't thus this ugly hack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and even more so, it should ignore things in headings... but for some god forsaken reason it didn't here
Preview for this PR was built for commit |
rework Capitalization rules
turn it back on