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.

Sign up for GitHub

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 multiple small (but sometimes serious) issues #1144

Merged
merged 4 commits into from
Jul 26, 2021
Merged

Fix multiple small (but sometimes serious) issues #1144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10f3b05
Fix documentation build failure
oturpe Jul 25, 2021
98ec617
Fix corrupted javascript
oturpe Jul 25, 2021
531a9bd
Display error correctly when Test Check fails
oturpe Jul 25, 2021
ffad6dd
Add pull request news entry
oturpe Jul 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions anitya/templates/project_new.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ <h1>{{ context }} project</h1>
"'https://packagist.org/packages/phpunit/php-code-coverage' " +
"this field should contain 'phpunit/php-code-coverage'.");
};
if (str == 'Sourceforge') || (str == 'Sourceforge (git)){
if ((str == 'Sourceforge') || (str == 'Sourceforge (git)')){
$("label[for='version_url']").text('Sourceforge name');
$('#version_url').prop('title', "Use this field if the name used " +
"on the rss page of the project differs from the name of the project." );
Expand Down Expand Up @@ -379,7 +379,7 @@ <h1>{{ context }} project</h1>
$(btn).show(); $(btn + "-spinner").hide();
alert(
'Unable to retrieve the latest version from upstream!\n'
+ 'ERROR: ' + res.responseJSON );
+ 'ERROR: ' + JSON.stringify(res.responseJSON) );
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not that familiar with javascript, so this will be probably my error. Thanks for fixing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are not the only one 😆
This is a very common mistake in error handlers.
The default string resulting from adding an object to string is the useless [Object object]
and it is even easier to accidentally write that kind of code
because e.g. console.log(object) actually prints something useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's why people are using some JavaScript replacements on serious projects?

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to get rid of Javascript entirely, but I'm not frontend developer, so this is not my cup of tea.

}
});
};
Expand Down
4 changes: 2 additions & 2 deletions docs/user-guide.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ The backends available are:
You need to provide **Sourceforge name** if the name on RSS feed is different then the
project name on Sourceforge.

* **Sourceforge (git) for projects hosted on
`sourceforge.net <https://sourceforge.net>`_
* **Sourceforge (git)** for projects hosted on
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised this wasn't caught by the tests. It was recently added in #1134.

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 was too. I looked at it again, Zuul configuration is wrong,
job anitya-tox-docs is actually running Bandit.
Probably just a copy paste error and easy to fix?

Otherwise, Vagrant uses the Makefile in the docs folder
while Zuul runs some commands embedded inside tox.ini.
That could cause varying results, too.

- job:
    name: anitya-tox-docs
    ...
    parent: tox
    run: ci/test-tox.yaml
    vars:
      tox_env: bandit
    ...

Copy link
Contributor

Choose a reason for hiding this comment

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

This really looks like a copy paste error on my side. I will fix it. I will also unify how the docs are built in vagrant and in tox.

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at the makefile and the docs tox target and they are almost identical, the tox one is even running ./generate_db_schema. So I would just fix the zuul file.

`sourceforge.net <https://sourceforge.net>`__

Sourceforge (git) retrieves the git tags in case a project hosted on sourceforge has them.
When a url to get the tags from is not provided, the sourceforge homepage of a project
Expand Down
1 change: 1 addition & 0 deletions news/PR1144.bug
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix documentation and javascript issues