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

Workflow list #11

Merged
merged 2 commits into from Jul 31, 2018
Merged

Workflow list #11

merged 2 commits into from Jul 31, 2018

Conversation

Sinclert
Copy link
Member

@Sinclert Sinclert commented Jul 19, 2018

First steps of the workflow-list implementation
Connect #10

@Sinclert Sinclert added this to the UI-Basics milestone Jul 19, 2018
@Sinclert Sinclert self-assigned this Jul 19, 2018
@Sinclert Sinclert force-pushed the workflow-list branch 7 times, most recently from 2f1fa2b to 64625cd Compare July 20, 2018 08:52
@Sinclert
Copy link
Member Author

Sinclert commented Jul 23, 2018

Update: I am encountering some problems while trying to access the API. The error is related with the Access-Control-Allow-Origin. I have encounter this problem both in Safari and Chrome, using 2 separate methods: the default fetch, and the library reqwest.

UPDATE: Solved

@Sinclert Sinclert force-pushed the workflow-list branch 8 times, most recently from 9f77751 to 77a8a7a Compare July 24, 2018 14:40
@diegodelemos
Copy link
Member

@reanahub/developers I think we should impose some format rules, I have found https://standardjs.com/ to be really useful after researching a bit about JavaScript formatters. It is not only the absence of configuration but also how easy it is to use from the CLI and integration with editors (emacs, VS Code...).

@tiborsimko
Copy link
Member

What about ESLint? It comes with AirBnB, Google, React styles, so that one could opt for a more popular style (AirBnB over Standard) or even use the same style rules that the React project uses. Here are some pages to read and ponder:

@ioannistsanaktsidis
Copy link
Member

ioannistsanaktsidis commented Jul 25, 2018

@reanahub/developers in CAP we are using this one https://prettier.io/ . We have set it up to trigger the moment you commit the files. It is working really good in case you want to check!

@diegodelemos
Copy link
Member

diegodelemos commented Jul 26, 2018

I would keep it simple and use standard (which we are already using in Invenio and is close to Python style) with no configuration if possible, only if really necessary (the only thing that I am not personally really happy with in standard defaults it is that it does not allow trailing commas, which in my opinion are necessary to simplify diffs). My second pick would be https://prettier.io/

I kind of agree with prettier/prettier#40 (comment).

@Sinclert
Copy link
Member Author

Sinclert commented Jul 26, 2018

I have been testing Standard, some comments:

  1. It is not native compatible with ES2017+ 'experimental features'. Even if this sounds exotic, it is something very common to have. For example:
    screen shot 2018-07-26 at 14 25 22
    It is easy to make it compatible with the new syntax. A package called babel-eslint can be installed and added to the parsing option to make it compatible (source):
    standard --parser babel-eslint

  2. I have found some parsing inconsistencies while parsing the JSX tags inside a render() functions. However, it doesn't happen in all the cases.

Regarding Prettier, some comments:

  1. It has some configuration level over CLI (source). I was gonna suggest to use the --use-tabs flag, for instance.

  2. Haven't encounter any problems with neither ES2017+ 'experimental syntax' nor JSX code inside a render() function.

Regarding ESlint with the AirBnb configuration, some comments:

  1. Exactly as Standard, ES2017+ syntax is not natively supported. It can be supported by adding the experimentalObjectRestSpread option in the config file (source).

  2. The default Vanilla / AirBnb config present some problems with JSX and React (commented here). They are not major, but requires to modify and build our own config file.

@tiborsimko
Copy link
Member

Looks like Prettier may be the most advantageous; let's see IRL tomorrow?

work correctly both with client-side routing and a non-root public URL.
Learn how to configure a non-root public URL by running `npm run build`.
-->
<title>React App</title>
Copy link
Member

Choose a reason for hiding this comment

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

REANA instead of React App.

@@ -0,0 +1,15 @@
{
"short_name": "Reana-UI",
Copy link
Member

Choose a reason for hiding this comment

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

Capital letters REANA here and everywhere else

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the name of the package still needs to be reana-ui, right?

Copy link
Member

Choose a reason for hiding this comment

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

* Transforms millisecond into a 'HH MM SS' string format
*/
static msToTime(millis) {
let seconds = Math.floor((millis / 1000) % 60);
Copy link
Member

Choose a reason for hiding this comment

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

what about using the Date object? something like

let string_date = "2018-07-30T15:52:44";
let date_obj = new Date(string_date)
let seconds = date_obj.getSeconds()
let minutes = date_obj.getMinutes()
let hours = date_obj.getHours()

Copy link
Member Author

@Sinclert Sinclert Jul 31, 2018

Choose a reason for hiding this comment

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

That way of operating is used to plot the creation date. However, for the Duration field, we should compare every date entity (secs, mins, hours, days, months, year).

Because of this reason, I transformed the date into a UNIX stamp number, and compute the difference between Date.now() and that number.

@Sinclert Sinclert force-pushed the workflow-list branch 3 times, most recently from 7ae33ef to df2c1a4 Compare July 31, 2018 07:47
run-tests.sh Outdated
@@ -21,4 +21,4 @@
# submit itself to any jurisdiction.

sphinx-build -qnN docs docs/_build/html
prettier reana-ui/src/**/*.js --list-different
#prettier reana-ui/src/**/*.js --list-different
Copy link
Member

Choose a reason for hiding this comment

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

Can you uncomment so we see if Travis is happy?

@@ -0,0 +1,117 @@
// In production, we register a service worker to serve assets from local cache.
Copy link
Member

Choose a reason for hiding this comment

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

I would remove this, we are not going to use Service Workers now, let see what happens after first implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean removing the whole file? or just that line?


import React, { Component } from "react";
import { Segment, Image, Button, Menu, Icon } from "semantic-ui-react";
import LogoImg from "../images/logo.svg";
Copy link
Member

Choose a reason for hiding this comment

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

It may be good to call the logo logo-reana.svg as we have in other repositories... Would be more comfortable for finding and updating files.

import TableActions from "./TableActions";
import _ from "lodash";

const URL = "http://reana-dev.cern.ch/api/workflows?";
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to have this hardcoded? If so, perhaps create config.js in the toplevel?

* Initial component structure (closes reanahub#10).

* Data fetching from REANA-server URL.

* Dynamic workflow actions.

* Global interface Header.
@diegodelemos diegodelemos merged commit 460b20d into reanahub:master Jul 31, 2018
@Sinclert Sinclert deleted the workflow-list branch July 31, 2018 14:54
@tiborsimko tiborsimko added this to Done in UI-Basics Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
UI-Basics
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants