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

docs: Fix invalid JSON in Stream Maps page and add meltano.yml tabs #1756

Merged
merged 13 commits into from
Jun 21, 2023

Conversation

mjsqu
Copy link
Contributor

@mjsqu mjsqu commented Jun 8, 2023

Resolves #797


📚 Documentation preview 📚: https://meltano-sdk--1756.org.readthedocs.build/en/1756/

mjsqu added 2 commits June 8, 2023 13:50
- Also removed trailing commas in list/dict objects to make the JSON syntactically valid
- Removed all comments from JSON and added to meltano.yml tab
@mjsqu mjsqu requested review from a team as code owners June 8, 2023 05:13
@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #1756 (fbf40d7) into main (d27f8ad) will not change coverage.
The diff coverage is n/a.

❗ Current head fbf40d7 differs from pull request most recent head c97a6a2. Consider uploading reports for the commit c97a6a2 to get more accurate results

@@           Coverage Diff           @@
##             main    #1756   +/-   ##
=======================================
  Coverage   86.12%   86.12%           
=======================================
  Files          59       59           
  Lines        4944     4944           
  Branches      809      809           
=======================================
  Hits         4258     4258           
  Misses        485      485           
  Partials      201      201           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mjsqu mjsqu changed the title docs: fix incompatible JSON in Stream Maps page and add meltano.yml tabs docs: fix invalid JSON in Stream Maps page and add meltano.yml tabs Jun 8, 2023
@mjsqu
Copy link
Contributor Author

mjsqu commented Jun 8, 2023

Quick link to the changed page:
https://meltano-sdk--1756.org.readthedocs.build/en/1756/stream_maps.html

@mjsqu
Copy link
Contributor Author

mjsqu commented Jun 8, 2023

Just shifted the meltano.yml tabs first, makes more sense as the tabs with the comments are displayed first - the JSON without comments can be viewed optionally

@pnadolny13
Copy link
Contributor

@mjsqu these tabs look so good, thanks for the PR!

The only thing I'm still unclear on is null handling in the meltano.yml files. I know theres an option to include __NULL__ or I guess in my case I ended up leaving it empty to represent null instead of the actual null keyword. @edgarrmondragon can you clarify the best way to represent these in the docs?

@edgarrmondragon edgarrmondragon changed the title docs: fix invalid JSON in Stream Maps page and add meltano.yml tabs docs: Fix invalid JSON in Stream Maps page and add meltano.yml tabs Jun 19, 2023
pyproject.toml Outdated Show resolved Hide resolved
docs/stream_maps.md Outdated Show resolved Hide resolved
docs/stream_maps.md Outdated Show resolved Hide resolved
docs/stream_maps.md Outdated Show resolved Hide resolved
docs/stream_maps.md Outdated Show resolved Hide resolved
docs/stream_maps.md Outdated Show resolved Hide resolved
docs/stream_maps.md Outdated Show resolved Hide resolved
docs/stream_maps.md Outdated Show resolved Hide resolved
docs/stream_maps.md Outdated Show resolved Hide resolved
@edgarrmondragon
Copy link
Collaborator

@mjsqu these tabs look so good, thanks for the PR!

The only thing I'm still unclear on is null handling in the meltano.yml files. I know theres an option to include __NULL__ or I guess in my case I ended up leaving it empty to represent null instead of the actual null keyword. @edgarrmondragon can you clarify the best way to represent these in the docs?

@pnadolny13 good catch! It's probably just better to use __NULL__ in all the meltano.yml examples to save users any potential issues around Meltano's handling of null values.

@edgarrmondragon
Copy link
Collaborator

Thanks @mjsqu! I've suggested some changes.

Co-authored-by: Edgar R. M. <edgarrm358@gmail.com>
@mjsqu
Copy link
Contributor Author

mjsqu commented Jun 20, 2023

Having some problems running poetry lock:
image

Co-authored-by: Edgar R. M. <edgarrm358@gmail.com>
@mjsqu
Copy link
Contributor Author

mjsqu commented Jun 20, 2023

Uh - I thought a poetry lock run was suggested, but I can't find that comment now 😆

@edgarrmondragon
Copy link
Collaborator

Uh - I thought a poetry lock run was suggested, but I can't find that comment now 😆

Lol, it's here: #1756 (comment)

Having some problems running poetry lock: image

Oh, bummer. The tabs plugin requires Python 3.8+ 😭. That means #1659 is a blocker for this.

@edgarrmondragon
Copy link
Collaborator

Oh, bummer. The tabs plugin requires Python 3.8+ 😭. That means #1659 is a blocker for this.

On second thought, RTD uses Python 3.11 to build the docs so using an env marker for the dependency should be fine.

@edgarrmondragon edgarrmondragon enabled auto-merge (squash) June 21, 2023 19:42
@edgarrmondragon edgarrmondragon merged commit c6a8933 into meltano:main Jun 21, 2023
24 checks passed
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.

Example has invalid JSON
3 participants