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

Replace www erlang links #68

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

eksperimental
Copy link
Contributor

First take on #66, let's see what Netlify finds

@netlify
Copy link

netlify bot commented Jan 19, 2022

❌ Deploy Preview for erlang-org failed.

🔨 Explore the source changes: 4a60273

🔍 Inspect the deploy log: https://app.netlify.com/sites/erlang-org/deploys/61e86b6dc288e400088bba06

@eksperimental
Copy link
Contributor Author

@garazdawi please have a look at the changes in commit 5df2f80

Copy link
Contributor

@garazdawi garazdawi left a comment

Choose a reason for hiding this comment

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

doc/search         /doc

Is actually wrong in the redirect and should be removed. I hadn't noticed it as netlify does not redirect any URLs that it can serve, and /doc/search does exist. So 5df2f80 should not be done.

_news/101.md Outdated


You can also read the documentation on-line here: 
(see the Release Notes mentioned above for release notes which 
are not updated in the doc, but the new functionality is)

[https://www.erlang.org/doc/](/doc/)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

_news/105.md Show resolved Hide resolved
@@ -44,7 +44,7 @@ Some highlights of the release are:
(see the Release Notes mentioned above for release notes which
are not updated in the doc, but the new functionality is)

<https://www.erlang.org/doc>
Copy link
Contributor

Choose a reason for hiding this comment

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

this is incorrect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this still incorrect?

_news/107.md Outdated
@@ -23,9 +23,9 @@ Some highlights of the release are:

You can find the Release Notes with more detailed info at

<https://www.erlang.org/download/otp_src_19.1.readme>
Copy link
Contributor

Choose a reason for hiding this comment

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

keep the www

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it is redirecting to erlang.org

Copy link
Contributor Author

@eksperimental eksperimental Jan 19, 2022

Choose a reason for hiding this comment

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

doc/search         /doc

Is actually wrong in the redirect and should be removed. I hadn't noticed it as netlify does not redirect any URLs that it can serve, and /doc/search does exist. So 5df2f80 should not be done.

So should it discard the two commits that replace /doc/search ?

Copy link
Contributor

Choose a reason for hiding this comment

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

But it is redirecting to erlang.org

www.erlang.org should have better uptime than erlang.org, so I would like to have links pointing to it as a first step. That way if we ever move any of the contents from erlang.org to www.erlang.org we will not have to rely on erlang.org to redirect us to the correct place.

Copy link
Contributor

Choose a reason for hiding this comment

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

So should it discard the two commits that replace /doc/search ?

yes. Feel free to remove the redirect rule from _redirects as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. I will do it in a different PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it is redirecting to erlang.org

www.erlang.org should have better uptime than erlang.org, so I would like to have links pointing to it as a first step. That way if we ever move any of the contents from erlang.org to www.erlang.org we will not have to rely on erlang.org to redirect us to the correct place.

Should this apply to everything, or just downloads?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean, should this apply to all the first set of rules in _redirects ?

Copy link
Contributor Author

@eksperimental eksperimental Jan 19, 2022

Choose a reason for hiding this comment

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

Ok. I have left the _redirects as the single source of truth, nothing in the site links to https://erlang.org now
I think it reduces the cognitive overhead A LOT

_news/107.md Outdated


You can also read the documentation on-line here:
(see the Release Notes mentioned above for release notes which
are not updated in the doc, but the new functionality is)

[https://www.erlang.org/doc/](/doc/)
Copy link
Contributor

Choose a reason for hiding this comment

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

incorrect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the display link you still be https://www.erlang.org/... ?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, exactly. The link should point to /some/thing/, but what is displayed should be the full path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

_news/19.md Outdated
@@ -11,7 +11,7 @@ visible: "true"
article_type_id: "3"
---

We are aware that parts of erlang.org need improvement. For example <https://www.erlang.org/article/tag/examples> and <https://www.erlang.org/course/course.html> are outdated. We would like to see a number of small code examples for beginners. The purpose of these examples is to provide an attractive and useful introduction for people who are interested in adopting the Erlang programming language. 
We are aware that parts of erlang.org need improvement. For example </article/tag/examples> and <https://erlang.org/course/course.html> are outdated. We would like to see a number of small code examples for beginners. The purpose of these examples is to provide an attractive and useful introduction for people who are interested in adopting the Erlang programming language. 
Copy link
Contributor

Choose a reason for hiding this comment

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

/article/tag/examples was removed in the new erlang.org version, so keep this as a dead link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@@ -23,7 +23,7 @@ article_type_id: "3"

The release contains many changes; thus, some unexpected incompatibilities or issues may have slipped through our tests. Please try to build and run your current products/applications and let us know about any problems.

Note! The new datatype MAP is not properly documented yet but the EEP 43 https://www.erlang.org/eeps/eep-0043.html will provide a good start. Also note that it is a limited implementation of maps implemented so far:
Copy link
Contributor

Choose a reason for hiding this comment

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

incorrect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@eksperimental eksperimental force-pushed the replace_www_erlang_links branch 2 times, most recently from e92b205 to 2e7bbd9 Compare January 19, 2022 18:18
@eksperimental eksperimental force-pushed the replace_www_erlang_links branch from fd43031 to ee371c2 Compare January 19, 2022 18:41
eksperimental added a commit to eksperimental-forks/erlang-org that referenced this pull request Jan 19, 2022
…at redirect to https://erlang.org

Replace
   "(https:\/\/erlang.org\/)(download|~|course|documentation|mailman|mailman-icons|pipermail)\/"

with
    "https:\/\/www.erlang.org\/\2\/"

See discussion: erlang#68 (comment)
@eksperimental
Copy link
Contributor Author

@garazdawi please have a look now.
What I have done is link everywhere to https://www.erlang.org and the _redirects file is the single source of truth to redirect to https://erlang.org.

@eksperimental eksperimental force-pushed the replace_www_erlang_links branch from 3a28f75 to 4a60273 Compare January 19, 2022 19:50
Copy link
Contributor

@garazdawi garazdawi left a comment

Choose a reason for hiding this comment

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

I've not gone though all the changes, but I've added notes on a couple so that you can see what type of changes need to be made.

@@ -40,8 +40,8 @@ For instructions on how to run with vscode devcontainers see: <https://code.visu

Most pages are either html or markdown pages so they can be edited directly. They
are located in the at the same place as the URL. So, for instance, the `/about` URL
is implemented by [/about.md](/about.md) and `/community/euc` is implemented in
[/community/euc](/community/euc.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not have the erlang.org prefix as it is a link in the github repo

@@ -40,7 +40,7 @@
{% endfor %}
</ul>
<small>Older releases and a file containing MD5 checksums for all files in the
<a href="https://erlang.org/download">download directory</a>,
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be rewritten


Download the new release from the [download page](https://erlang.org/download.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be rewritten

@@ -44,7 +44,7 @@ Some highlights of the release are:
(see the Release Notes mentioned above for release notes which
are not updated in the doc, but the new functionality is)

<https://www.erlang.org/doc>
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this still incorrect?

@@ -21,14 +21,14 @@ Some highlights for 19.2

You can find the README and the full listing of changes for this service release at

  https://www.erlang.org/download/otp_src_19.2.readme
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be rewritten, but could have a < tag around the link

Comment on lines -29 to -31
  https://www.erlang.org/download/otp_src_19.2.tar.gz
  https://www.erlang.org/download/otp_win32_19.2.exe
  https://www.erlang.org/download/otp_win64_19.2.exe
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be rewritten, but could have a < tag around the link

@@ -37,11 +37,11 @@ For installation instructions please consult the README file that is part of th
The Erlang/OTP source can also be found at GitHub on the official Erlang
repository, https://github.com/erlang/otp with tag OTP-19.2

The on-line documentation can be found at: https://www.erlang.org/doc/
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be rewritten, but could have a < tag around the link

Comment on lines -43 to -44
  https://www.erlang.org/download/otp_doc_html_19.2.tar.gz
  https://www.erlang.org/download/otp_doc_man_19.2.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be rewritten, but could have a < tag around the link

@@ -16,7 +16,7 @@ article_type_id: "3"

See the release notes in the [readme file](https://www.erlang.org/download/otp_src_R14B04.readme).

Download the new release from the [download page](https://www.erlang.org/download.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be rewritten


Online documentation can be browsed here:
[https://erlang.org/doc/search/](/doc/search/)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should only add the www prefix, the link should be as is.

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.

2 participants