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 up some minor spicy tutorial docs #270

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

evantypanski
Copy link
Contributor

This is just what I found going through the tutorial myself

  1. The initial command zeek - N Zeek::Spicy has an extra space so that command is invalid
  2. Line numbers are wrong because the tftp.evt file added two new lines at the top I think. Its current state looks like:

Screenshot 2024-08-07 at 1 51 58 PM

It should (and does after this) look like:

Screenshot 2024-08-07 at 1 51 48 PM

  1. Added an import spicy; mention since I personally prefer everything needed to compile a program being somewhere in the tutorial. It is in the complete grammar at the end of the spicy tutorial, though, but I prefer a mention in the zeek integration too (since it was never mentioned in the actual spicy tutorial)

@evantypanski evantypanski requested a review from rsmmr August 7, 2024 18:03
@timwoj
Copy link
Member

timwoj commented Aug 7, 2024

It should (and does after this) look like:

Screenshot 2024-08-07 at 1 51 48 PM

This is going to be wrong again in about 30 minutes after I remove the deprecated port field. It was marked as deprecated in 7.0 and we're removing support entirely in 7.1.

Copy link
Member

@timwoj timwoj left a comment

Choose a reason for hiding this comment

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

This looks fine to me. We'll need to make the associated deprecation fixes and update the line numbers again for tftp.evt, but that can be separate from this PR.

Copy link
Member

@rsmmr rsmmr left a comment

Choose a reason for hiding this comment

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

Nice, thanks! Just one small tweak for clarity.

devel/spicy/tutorial.rst Outdated Show resolved Hide resolved
@evantypanski evantypanski force-pushed the topic/etyp/spicy-zeek-minor-fixes branch from 0fcb227 to 64efa4a Compare August 8, 2024 09:54
@evantypanski evantypanski requested a review from rsmmr August 8, 2024 14:07
@rsmmr rsmmr merged commit f450f80 into master Aug 8, 2024
10 checks passed
@rsmmr rsmmr deleted the topic/etyp/spicy-zeek-minor-fixes branch August 8, 2024 14:49
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