#129 Module to display vps info

Merged
jocelyn merged 6 commits from ARN/vps into FFDN/master 6 years ago
ljf commented 7 years ago

This module allow to display vps info like IPs, vnc console or fingerprint.

It works.

TODO:

  • Documentation
  • Bug : "None" displayed to the user (if no IPs are set)
This module allow to display vps info like IPs, vnc console or fingerprint. It works. TODO: - Documentation - Bug : "None" displayed to the user (if no IPs are set)
jocelyn commented 7 years ago
Owner

@ljf there are missing test data (in offers.json).

That lines in your PR refers to test data : 

self.offer = Offer.objects.filter(configuration_type="VPSConfiguration")[0]

That seem to be missing (in offers.json ?).

@ljf there are missing test data (in offers.json). That lines in your PR refers to test data :  ``` self.offer = Offer.objects.filter(configuration_type="VPSConfiguration")[0] ``` That seem to be missing (in offers.json ?).
jocelyn commented 7 years ago
Owner

@ljf bump

@ljf bump
ljf commented 6 years ago
Collaborator

I fixed unit tests

For me it's ok to be merged

I fixed unit tests For me it's ok to be merged
alexAubin commented 6 years ago
Collaborator

Bumpitybump,

It would be nice to merge this :/

As far as I remember, Jocelyn was worried that this code should be factorized because there's a lot in common between VPS, VPN and Housing ... which I agree with. On the other hand, I doubt that we'll have manpower for this anytime soon and it's already working and is already a dependency for some other PRs if I remember correctly :s ...

What are your thoughts on this ?

Bumpitybump, It would be nice to merge this :/ As far as I remember, Jocelyn was worried that this code should be factorized because there's a lot in common between VPS, VPN and Housing ... which I agree with. On the other hand, I doubt that we'll have manpower for this anytime soon and it's already working and is already a dependency for some other PRs if I remember correctly :s ... What are your thoughts on this ?
jocelyn commented 6 years ago
Owner

Fair enough. Lets not stay stuck here. I suggest that you merge and just take the time to commit big fat warning comments urging anyone starting to work on those parts to start by factorizing the code.

But I'd be verry happy if we avoid doing the same again in the future :-).

WDYT ?

Fair enough. Lets not stay stuck here. I suggest that you merge and just take the time to commit big fat warning comments urging anyone starting to work on those parts to start by factorizing the code. But I'd be verry happy if we avoid doing the same again in the future :-). WDYT ?
jocelyn commented 6 years ago
Owner

Soyons fous :-)

Soyons fous :-)
This pull request has been merged successfully!
Sign in to join this conversation.
No Milestone
No assignee
3 Participants
Loading...
Cancel
Save
There is no content yet.