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

FEATURE : Portfolio duplication (closes #190) #198

Draft
wants to merge 3 commits into
base: v7
Choose a base branch
from

Conversation

AxelRagobert
Copy link

@AxelRagobert AxelRagobert commented Jun 13, 2019

These commits allow you to duplicate a portfolio if you are connected. It also makes porphyry more responsive.
However to be able to test the duplication, you have to change the url from localhost:3000 to portfolioName.local:3000 and modify your /etc/hosts file to match these kind URIs with 127.0.0.1. You also have to run a small reverse-proxy which is a the root of the project.

Please check that your pull request is correct:

  • Each commit:
    • corresponds to a contribution that should be notified to users,
    • does not generate new errors or warnings at compile or test time,
    • must be attributed to its real authors (with correct GitHub IDs and correct syntax for multiple authors).
  • The title of a commit should:
    • begin with a contribution type
      • FEATURE for a behaviour allowing a user to do something new,
      • FIX for a behaviour which has been changed in order to meet user’s expectations,
      • TEST when it concerns an acceptance test,
      • PROCESS for a change in the way the software is built, tested, deployed,
      • DOC when it concerns only internal documentation (however it is better to combine it with the contribution that required this documentation change),
    • be followed by a colon (:) with one space after and no space before,
    • be followed by a title (written in English) as short, as user-centered and as explicit as possible
      • If it is a feature, the title must be the user action (beginning with a verb, and please not manage),
      • If it is a fix, the title must describe the intended behavior (with should).
    • ends with a reference to the corresponding ticket with the following syntax:
      • (closes #xx) if xx is a feature ticket (and the commit is a complete implementation),
      • (fixes #xx) if xx is a fix ticket (and the commit is a complete fix),
      • (see #xx) otherwise,
  • Each committed line is:
    • useful (it would not work if removed)
      • if it is a comment line, its information could not be conveyed by better variables and function naming, better code structuring, or better commit message,
    • related to this very contribution (feature, fix...),
    • in English (with the exception of Gherkin scenarios in French and resulting steps),
    • without any typo in variable, class or function names,
    • correctly indented (spaces rather than tabs, same number of characters as in the rest of the file).

@benel
Copy link
Member

benel commented Jun 13, 2019

Thank you for this nice contribution.

Could you please restructure the history into 2 commits (one for the FEATURE and one for the TEST) as specified by the rules?

... especially those about other contributors (if any) and ticket reference.

@AxelRagobert
Copy link
Author

Done !

@AxelRagobert
Copy link
Author

Understanding your note of this morning, I've pended my duplication test. Here are some manipulations you'll have to make to make it work. As a reminder, this issue is due to the CORS settings.

Change by in package.json
"start": "react-scripts start"
by
"start": "HOST=vitraux.local react-scripts start"

Change in config/config.json
{ "user": "vitraux", "services": [ "http://argos2.test.hypertopic.org", "http://steatite.hypertopic.org" ] }
by
{ "services": [ "http://localhost", "http://steatite.hypertopic.org" ] }

Launch the reverse proxy in the folder reverse-proxy with sudo npm start.

Then the duplication test should succeed.

@AxelRagobert
Copy link
Author

AxelRagobert commented Jun 21, 2019

It seems that npm install doesn't work on Travis. It's maybe a server issue..

@AxelRagobert AxelRagobert reopened this Jun 21, 2019
@benel benel added the Draft label Mar 19, 2020
@benel benel marked this pull request as draft April 29, 2020 09:48
@benel benel removed the Draft label Apr 29, 2020
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.

3 participants