Closed Bug 1238170 Opened 8 years ago Closed 8 years ago

Improve dev server room routing code clarity

Categories

(Hello (Loop) :: Client, defect, P3)

Product:
Hello (Loop)
Component:
Client
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: dmosedale, Assigned: dmosedale)

References

Details

Attachments

(2 files, 1 obsolete file)

[loop] dmose:local-server-content > mozilla:master
39 bytes, text/x-github-pull-request
Details | Review
Link to Github pull-request: https://github.com/mozilla/loop/pull/62
39 bytes, text/x-github-pull-request
standard8
: review+
Details | Review 
      No description provided.
https://github.com/mozilla/loop/commit/4c69c769e3b007b7e5aa9960ca2342c4f3ea0aac#diff-8d44631bfd02d64ee75348ab92f466c0L106 regressed the ability to run a local loop-server pointing to a local dev server, which makes debugging some key standalone changes (eg CSP changes) impossible.
Assignee: nobody → dmose
Blocks: 1212428
Rank: 20
Priority: -- → P2

        Attachment #8705869 -
        Attachment is obsolete: true

        Attachment #8705875 -
        Flags: review?(standard8)

        Attachment #8705875 -
        Flags: review?(edilee)

        Attachment #8705875 -
        Flags: review?(dcritchley)

    
    

    
  
Comment on attachment 8705875 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop/pull/62

If I understand this right, then I don't want to do this.

What I think the PR is allowing for is /content/<token> as well as /<token>.

When we set up the new standalone server I purposely decided to drop the /content part of the url. This was to make us closer to the real production system, and avoid a lot of the complicated hacks that we had in place.

Unfortunately I didn't do a very good job of remembering to communicate that.

Hence, I really don't think we should allow /content, and rather update the loop-server default configs and our example dev config to drop the /content from the urls (and inform all devs, which I'll send out on Monday).

        Attachment #8705875 -
        Flags: review?(standard8)

        Attachment #8705875 -
        Flags: review?(edilee)

        Attachment #8705875 -
        Flags: review?(dcritchley)

        Attachment #8705875 -
        Flags: review-

    
    

    
  
Comment on attachment 8705875 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop/pull/62

Updated to not add extra routing, but keep clarity.

        Attachment #8705875 -
        Flags: review- → review?(standard8)
Summary: Fix dev server regression to handle local server calls → Improve dev server room routing code clarity
Rank: 20 → 35
Priority: P2 → P3

    
    

    
  
Comment on attachment 8705875 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop/pull/62

Looks good.

        Attachment #8705875 -
        Flags: review?(standard8) → review+

    
    

    
  
https://github.com/mozilla/loop/commit/42ab05c213ffe56b6834b19428b837dadc309252
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: