Skip to content

Conversation

@chefleo
Copy link
Contributor

@chefleo chefleo commented Dec 26, 2020

Makes progress on #585

@tunetheweb tunetheweb added the translation world wide web label Dec 27, 2020
@tunetheweb tunetheweb added this to the 2020 Content Translation milestone Dec 27, 2020
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Thanks for this @chefleo but as mentioned previously we can't merge this until we have the base templates translated into Italian too:

  • en/base.html
  • en/base_chapter.html
  • en/error.html
  • en/2020/base.html
  • en/2020/index.html
  • en/2020/table_of_contents.html

Most of these are very small and just contain phrases used to build the site, though the first one is larger.

Also made some other suggestions below. Will do a more thorough review when they are addressed.

@Zizzamia Zizzamia self-requested a review December 31, 2020 22:04
@Zizzamia
Copy link
Contributor

Zizzamia commented Jan 1, 2021

Btw before merging let's have also @giopunt review 😁

@tunetheweb tunetheweb marked this pull request as draft January 3, 2021 17:56
@tunetheweb
Copy link
Member

Converting to Draft as not ready to be merged until we have base templates translated too.

@chefleo chefleo marked this pull request as ready for review January 7, 2021 14:38
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

A few more things I spotted preventing the chapter from generating

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Woo hoo! It's now generating the chapter.

Did a thorough review and made some comments. Hopefully most are very quick to fix.

Thanks for working on all this! Will make future chapters a lot easier!

@Zizzamia
Copy link
Contributor

Ciao @bazzadp, I heard from @giopunt that he wasn't able to comment or review this PR. Is it possible because he is not part of the HTTPArchive org?

@tunetheweb
Copy link
Member

Ciao @bazzadp, I heard from @giopunt that he wasn't able to comment or review this PR. Is it possible because he is not part of the HTTPArchive org?

Probably. Just invited him there. We should add his profile to 2020.json as well if he makes a contribution so he is credited on our contributor page.

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

A couple of new attributes we recently added that need to be in the base templates and are causing the build to fail without them.

@tunetheweb
Copy link
Member

@chefleo I pointed out some more comments above that were unresolved. They were hidden under this "Load More..." button so you may have missed them:

image

Once you resolve those, and that last one I added there no to revert the lang on the error message, we are good to merge on my side. How are we looking on the Italian side? Still reviewing or happy to merge once I am?

@tunetheweb
Copy link
Member

tunetheweb commented Jan 11, 2021

@chefleo / @giopunt / @Zizzamia ,

I staged a version of this branch here:

So you can see what it will look like and see if you can spot any errors.

Zizzamia and others added 6 commits January 12, 2021 23:55
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
@Zizzamia
Copy link
Contributor

Sweeet, went over all the open comments left and I think we should be good.
Some translations might need a second PR, but I think we are in a good start. Myself and @giopunt can add more polish in future PRs.
Thank you @bazzadp and @giopunt for helping out @chefleo making this happen, was his first open source contribution just out of university. 👏

@tunetheweb
Copy link
Member

Sweeet, went over all the open comments left and I think we should be good.
Some translations might need a second PR, but I think we are in a good start. Myself and @giopunt can add more polish in future PRs.

Thanks @Zizzamia - think we're pretty good now. Will give @giopunt and @chefleo the morning to look over this and merge later today if I hear no reason not to. We can always fix things in subsequent PRs as you say.

Thank you @bazzadp and @giopunt for helping out @chefleo making this happen, was his first open source contribution just out of university. 👏

Well done @chefleo - promise they aren't all this hard! Languages are always harder than code IMHO. And, as the first Italian translation, you also got hit with all the base translations templates in this one. Future Italian translations won't have this and will instead benefit from your hard work here!

@tunetheweb tunetheweb merged commit 2c46fa0 into HTTPArchive:main Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

translation world wide web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants