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

SC documentation improvements #963

Merged
merged 21 commits into from
Feb 2, 2024
Merged

SC documentation improvements #963

merged 21 commits into from
Feb 2, 2024

Conversation

dg-concordium
Copy link
Contributor

@dg-concordium dg-concordium commented Jan 25, 2024

Purpose

Improve smart contract documentation based on comments from Kasper and some outstanding pull requests. Note that this does NOT include changes to the tutorials.

Changes

A number of sc files
Conf.py (redirects)
Requirements.txt because something broke

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

CLA acceptance

_Remove if not applicable.

By submitting the contribution I accept the terms and conditions of the
Contributor License Agreement v1.0

@dg-concordium dg-concordium marked this pull request as ready for review January 26, 2024 10:48
@limemloh limemloh added the preview Trigger a documentation preview for a PR. label Jan 30, 2024
Copy link
Collaborator

@limemloh limemloh 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! Nice work. I have some minor suggestions and discussions, but nothing blocking.

source/mainnet/net/guides/developer-page.rst Show resolved Hide resolved
source/mainnet/net/guides/developer-page.rst Outdated Show resolved Hide resolved
source/mainnet/net/guides/developer-page.rst Outdated Show resolved Hide resolved
source/mainnet/smart-contracts/general/introduction.rst Outdated Show resolved Hide resolved
source/mainnet/smart-contracts/guides/local-simulate.rst Outdated Show resolved Hide resolved
source/mainnet/smart-contracts/guides/setup-contract.rst Outdated Show resolved Hide resolved
source/mainnet/smart-contracts/guides/setup-tools.rst Outdated Show resolved Hide resolved
source/shared/setup-env.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@Bargsteen Bargsteen left a comment

Choose a reason for hiding this comment

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

Nice work 👍

source/mainnet/net/guides/developer-page.rst Outdated Show resolved Hide resolved
source/mainnet/net/guides/developer-page.rst Outdated Show resolved Hide resolved
source/mainnet/smart-contracts/general/contract-schema.rst Outdated Show resolved Hide resolved
source/mainnet/smart-contracts/general/introduction.rst Outdated Show resolved Hide resolved
source/mainnet/smart-contracts/guides/quick-start.rst Outdated Show resolved Hide resolved
source/shared/setup-env.rst Show resolved Hide resolved
dg-concordium and others added 10 commits February 1, 2024 10:39
Co-authored-by: Emil Holm Gjørup <eg@concordium.com>
Co-authored-by: Emil Holm Gjørup <eg@concordium.com>
Co-authored-by: Emil Holm Gjørup <eg@concordium.com>
Co-authored-by: Emil Holm Gjørup <eg@concordium.com>
Co-authored-by: Emil Holm Gjørup <eg@concordium.com>
Co-authored-by: Kasper Dissing Bargsteen <kb@concordium.com>
Co-authored-by: Kasper Dissing Bargsteen <kb@concordium.com>
@dg-concordium
Copy link
Contributor Author

I think I got all of the videos embedded, but please check to make sure that I got the correct ones in the correct places as the copy/paste got a bit confusing.

@Concordium Concordium deleted a comment from github-actions bot Feb 1, 2024
@dg-concordium dg-concordium added preview Trigger a documentation preview for a PR. and removed preview Trigger a documentation preview for a PR. labels Feb 1, 2024
@dg-concordium
Copy link
Contributor Author

All of the changes should be done. Please review @Bargsteen and @limemloh.

@Bargsteen
Copy link
Contributor

Bargsteen commented Feb 2, 2024

I noticed that the dimensions of the videos are fixed and they seem small at least on my screen.

image

I think we should remove the fixed dimensions (560x315) for all of our videos and add a CSS rule:

iframe {
   width: 100%;
   aspect_ratio: 16 / 9;
}

(We can also add a youtube class, but I don't think we'll use other iframes)

As that gives us this:

image

It also scales better on small devices.
We should also describe how to embed videos in our readme and explain that you should remove the fixed dimensions.

@Concordium Concordium deleted a comment from github-actions bot Feb 2, 2024
@dg-concordium dg-concordium removed the preview Trigger a documentation preview for a PR. label Feb 2, 2024
@dg-concordium
Copy link
Contributor Author

Once again, the changes are made. Please review @Bargsteen

@dg-concordium dg-concordium added the preview Trigger a documentation preview for a PR. label Feb 2, 2024
Copy link

github-actions bot commented Feb 2, 2024

PR Preview Action v1.4.6
Preview removed because the pull request was closed.
2024-02-02 12:48 UTC

Copy link
Contributor

@Bargsteen Bargsteen left a comment

Choose a reason for hiding this comment

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

Nice 👌

@dg-concordium dg-concordium merged commit e0cfddb into main Feb 2, 2024
2 checks passed
@dg-concordium dg-concordium deleted the sc-doc-improvements branch February 2, 2024 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Trigger a documentation preview for a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants