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

Update README.md #197

Merged
merged 4 commits into from
Jun 3, 2024
Merged

Update README.md #197

merged 4 commits into from
Jun 3, 2024

Conversation

mbifeld
Copy link
Member

@mbifeld mbifeld commented May 23, 2024

Updated IE installation and environment variables details. General doc formatting improvements.

mbifeld added 2 commits May 23, 2024 13:11
Added updated download options for IE CLI and fixed out of date instructions
@mbifeld mbifeld temporarily deployed to ScenarioTesting May 23, 2024 23:26 — with GitHub Actions Inactive
@mbifeld mbifeld requested a review from vmarcella May 23, 2024 23:27
@@ -169,19 +170,6 @@ jobs:
python3 main.py test README.md
```


## Use Executable Documentation for Interactive Documentation
Copy link
Member

Choose a reason for hiding this comment

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

Is interactive mode something that we're not suggesting for users to use anymore?

Copy link
Member Author

@mbifeld mbifeld May 25, 2024

Choose a reason for hiding this comment

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

It is something we're still suggesting but these details are already in the 'Modes of Operation' section

README.md Outdated
@@ -114,16 +106,25 @@ markdown. For example:
<!!--
```variables
Copy link
Member

Choose a reason for hiding this comment

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

I think we should do away with this method of declaring variables as variables declared in the codeblocks provide the best experience for both the authors of the documents and users executing the documents. This way of declaring variables is very hacky from an authoring perspective and introduces hidden state into the documents from the users perspective

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I think we can keep the functionality but don't have to call it out here.

README.md Outdated
```

CLI argument variables override environment variables declared within the markdown document,
which override variables set in a .ini file, which override preexisting environment variables.
Copy link
Member

Choose a reason for hiding this comment

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

Same as variable comment blocks, I think we should also deprecate ini files for variable declarations. I've not seen any doc authors use these and don't think they add much value to the authoring experience, but these aren't nearly as bad as variables declared inside HTML comments.

Copy link
Member

Choose a reason for hiding this comment

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

Another thing to note is that the INI files MUST have the same name as the scenario itself. So, if we are to keep them, we should make that abundantly clear if not already.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm impartial either way, removed for now to keep things more concise. Let's keep the functionality though assuming no effort is needed to maintain it. But there is a request to expand on the ini doc to install dependencies in #152 . Although this request would also be solved by pre-reqs.

Copy link
Member

Choose a reason for hiding this comment

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

IMO the INI file wouldn't be able to effectively solve #152 because there's no good way to resolve package names across various Linux distributions. Each distribution uses a different package management tool and each the name of the packages can be different across distributions although containing the same content as each other. Pre-requisites is definitely the preferred solution for this problem.

@mbifeld mbifeld temporarily deployed to ScenarioTesting May 25, 2024 01:03 — with GitHub Actions Inactive
@mbifeld mbifeld temporarily deployed to ScenarioTesting May 25, 2024 01:08 — with GitHub Actions Inactive
@vmarcella vmarcella self-requested a review June 3, 2024 20:27
Copy link
Member

@vmarcella vmarcella left a comment

Choose a reason for hiding this comment

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

Looks good to me now!

@mbifeld mbifeld added this pull request to the merge queue Jun 3, 2024
Merged via the queue into main with commit 2f1f50d Jun 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants